Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

3x3 matrix where eigenvalues fail to converge #3036

Open
gwhitney opened this issue Oct 2, 2023 · 1 comment
Open

3x3 matrix where eigenvalues fail to converge #3036

gwhitney opened this issue Oct 2, 2023 · 1 comment
Labels

Comments

@gwhitney
Copy link
Collaborator

gwhitney commented Oct 2, 2023

Describe the bug
Exercise 7.7.103 of https://web.uvic.ca/~tbazett/diffyqs/sec_multeigen.html produces "The eigenvalues failed to converge" error. I feel mathjs should be able to handle elementary textbook examples, especially for such a small matrix. Admittedly, this is a case where all three eigenvalues are equal, and the current method relies on differences of eigenvalues to converge, so it was sort of doomed, but really mathjs should have alternate methods to use in such a case.

To Reproduce
math.eigs([[2, 0, 0], [-1, -1, 9], [0, -1, 5]])

Note further that although this is a case of a defective matrix (the algebraic multiplicity of eigenvalue 2 is three, but its geometric multiplicity is only one), this error is independent of #2879: it occurs before eigenvalues have even been determined, and so the code to search for eigenvectors is never even invoked. If we can get mathjs to realize that 2 is an eigenvalue of algebraic multiplicity three, then the findEigenvectors code might (or might not) properly discover the unique eigenvector proportional to [0, 3, 1].

gwhitney added a commit to gwhitney/mathjs that referenced this issue Oct 2, 2023
  Previously, attempting to take the `eigs` of any defective matrix
  was doomed to fail in an attempt to solve a singular linear system.
  This PR detects the situation (as best as it can given the
  inherent numerical instability of the current methods used) and
  handles it. Note that in such cases, it's not possible to return
  a square matrix whose columns are the eigenvectors corresponding to
  the returned eigenvalues. In light of that fact and issue josdejong#3014, this
  PR also changes the return value of `eigs` so that the eigenvectors
  are passed back in a property `eigenvectors` which is an array of
  plain objects `{value: e, vector: v}`.

  Note that this PR makes the ancillary changes of correcting the
  spelling of the filename which was "realSymetric.js," and replacing
  the now-unnecessary auxiliary function "createArray" therein with
  `Array(size).fill(element)`. The rationale for performing these
  changes not strictly related to the issues at hand is that this
  file is rarely touched and with the level of maintenance hours we have
  at hand, it's more efficient to do these small refactorings in parallel
  with the actual bugfixes, which are orthogonal and so will not be
  obfuscated by this refactor. Note `git diff` does properly track the
  file name change.

  However, it also makes a potentially more pervasive change: in order for
  the numerically-sensitive algorithm to work, it changes the condition
  on when two very close (double) numbers are "nearlyEqual" from differing by
  less than DBL_EPSILON to differing by less than or equal to DBL_EPSILON.
  Although this may change other behaviors than the ones primarily being
  addressed, I believe it is an acceptable change because

  (a) It preserves all tests.
  (b) DBL_EPSILON is well below the standard config.epsilon anyway
  (c) I believe there are extant issues noting the odd/inconsistent
      behavior of nearlyEqual near 0 anyway, so I believe this will
      be overhauled in the future in any case. If so, the eigenvector
      computation will make a good test that a future nearlyEqual
      algorithm is working well.

  To be clear, the direct motivation for the change is that there are
  multiple cases in the eigenvector computation in which a coefficient
  that is "supposed" to be zero comes out to precisely DBL_EPSILON, which
  is fairly unsurprising given that these coefficients are produced by
  subtracting an eigenvalue from a diagonal entry of a matrix, which is
  likely to be essentially equal to that eigenvalue.

  As many tests of defective matrices as I could readily find by web
  searching have been added as unit tests (and one more in the typescript
  type testing). An additional case I found still fails, but in the
  _eigenvalue_ computation rather than the _eigenvector_ search, so that
  was deemed beyond the scope of this PR and has been filed as issue josdejong#3036.

  Resolves josdejong#2879.
  Resolves josdejong#2927.
  Resolves josdejong#3014.
@gwhitney gwhitney added the bug label Oct 3, 2023
@gwhitney
Copy link
Collaborator Author

gwhitney commented Oct 4, 2023

The situation will be much improved by the merging of #3037; there, one can go up to config.epsilon precision and beyond (although it doesn't make the eigenvalues any closer beyond that) and get the eigenvalues to match to within 6 parts in a million. That's still not very accurate for double precision arithmetic, and not accurate enough to realize there is only one eigenvector. So this still needs improvement someday, either by switching to the full Francis implicit QR iteration, or using better shifts, or both, or by using some other eigenvalue algorithm, or by using a better-optimized library rather than implementing eigenvectors directly within mathjs.

josdejong pushed a commit that referenced this issue Oct 5, 2023
* fix: Find eigenvectors of defective matrices

  Previously, attempting to take the `eigs` of any defective matrix
  was doomed to fail in an attempt to solve a singular linear system.
  This PR detects the situation (as best as it can given the
  inherent numerical instability of the current methods used) and
  handles it. Note that in such cases, it's not possible to return
  a square matrix whose columns are the eigenvectors corresponding to
  the returned eigenvalues. In light of that fact and issue #3014, this
  PR also changes the return value of `eigs` so that the eigenvectors
  are passed back in a property `eigenvectors` which is an array of
  plain objects `{value: e, vector: v}`.

  Note that this PR makes the ancillary changes of correcting the
  spelling of the filename which was "realSymetric.js," and replacing
  the now-unnecessary auxiliary function "createArray" therein with
  `Array(size).fill(element)`. The rationale for performing these
  changes not strictly related to the issues at hand is that this
  file is rarely touched and with the level of maintenance hours we have
  at hand, it's more efficient to do these small refactorings in parallel
  with the actual bugfixes, which are orthogonal and so will not be
  obfuscated by this refactor. Note `git diff` does properly track the
  file name change.

  However, it also makes a potentially more pervasive change: in order for
  the numerically-sensitive algorithm to work, it changes the condition
  on when two very close (double) numbers are "nearlyEqual" from differing by
  less than DBL_EPSILON to differing by less than or equal to DBL_EPSILON.
  Although this may change other behaviors than the ones primarily being
  addressed, I believe it is an acceptable change because

  (a) It preserves all tests.
  (b) DBL_EPSILON is well below the standard config.epsilon anyway
  (c) I believe there are extant issues noting the odd/inconsistent
      behavior of nearlyEqual near 0 anyway, so I believe this will
      be overhauled in the future in any case. If so, the eigenvector
      computation will make a good test that a future nearlyEqual
      algorithm is working well.

  To be clear, the direct motivation for the change is that there are
  multiple cases in the eigenvector computation in which a coefficient
  that is "supposed" to be zero comes out to precisely DBL_EPSILON, which
  is fairly unsurprising given that these coefficients are produced by
  subtracting an eigenvalue from a diagonal entry of a matrix, which is
  likely to be essentially equal to that eigenvalue.

  As many tests of defective matrices as I could readily find by web
  searching have been added as unit tests (and one more in the typescript
  type testing). An additional case I found still fails, but in the
  _eigenvalue_ computation rather than the _eigenvector_ search, so that
  was deemed beyond the scope of this PR and has been filed as issue #3036.

  Resolves #2879.
  Resolves #2927.
  Resolves #3014.

* refactor: remove comma that lint now doesn't like

* test: add a test for eigs with a precision argument

* feat: Use simple shifts in QR eigenvalue iterations that improve convergence

  Although we might want to use better shifts in the future, we might just
  use a library instead. But for now I think this:
  Resolves #2178.

  Also responds to the review feedback provided in PR #3037.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant