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

Allow eigs to only find eigenvalues, not eigenvectors #2180

Closed
cshaa opened this issue Apr 12, 2021 · 7 comments · Fixed by #3057
Closed

Allow eigs to only find eigenvalues, not eigenvectors #2180

cshaa opened this issue Apr 12, 2021 · 7 comments · Fixed by #3057

Comments

@cshaa
Copy link
Collaborator

cshaa commented Apr 12, 2021

The current implementation of eigs for complex matrices (see PR #1743) starts with finding eigenvalues and then proceeds to find eigenvectors. Since not everybody needs eigenvectors, a significant portion of time could be saved by skipping this part. This is one small step towards fixing #1175.

@josdejong
Copy link
Owner

👍 makes sense

@Priyaraj17
Copy link

May I work on this?

@cshaa
Copy link
Collaborator Author

cshaa commented May 22, 2021

Yes, definitely :)
The algorithms you'll want to use are in /src/function/matrix/eigs/. I'm not sure about the algorithm for real symmetric matrices, but the one for complex matrices has a boolean argument indicating whether it sould find eigenvectors or not, so the implementation should be fairly simple there.

@josdejong
Copy link
Owner

Thanks for your offer @Priyaraj17 👍

@cshaa cshaa mentioned this issue Jun 19, 2021
@gwhitney gwhitney changed the title Add a function that only finds eigenvalues, not eigenvectors Allow eigs to only find eigenvalues, not eigenvectors Oct 3, 2023
@gwhitney
Copy link
Collaborator

gwhitney commented Oct 3, 2023

Since the flag to only find eigenvalues is already there in src/function/matrix/eigs/complexEigs.js, it would only take a small change to the definition of the eigs() function to enable this. (The overhead of eigenvectors in the real symmetric case is much less, and so at least as a first pass you could just compute them and throw them away in that case.) So this would still be a potentially worthwhile change. Allowing the second argument to eigs to be a options object with properties precision (a number) and/or eigenvectors (a boolean, defaulting to true), but when it is just a number interpreting it as precision, would be a plausible non-breaking way to achieve this. Or possibly since we are about to break anyway (see #3032), would we want to just change the second argument of eigs() to have to be such an options object? Just a thought, I could do a quick PR in time for v12 if desired.

@gwhitney gwhitney mentioned this issue Oct 3, 2023
6 tasks
@josdejong
Copy link
Owner

Great idea!

It would be neat to change the syntax of eigs and complexEigs to have an object with options { precision?: number, eigenvectors?: boolean }. I'm also wondering about the second argument of complexEigs, the size of the matrix N, that could be easily determined inside the function itself.

Right now, complexEigs is not exposed in the public API, it is only used internally inside eigs, so we can change the syntax as we like. I would like to keep the arguments of eigs backward compatible so it's not a breaking change, and I think that is perfectly possible: then it has the following syntax:

eigs(x)
eigs(x, precision)
eigs(x, options) // options: { precision?, eigenvectors? }

gwhitney added a commit to gwhitney/mathjs that referenced this issue Oct 4, 2023
  For large matrices, the eigenvector computation can be noticeably expensive
  and so it's worthwhile to have a way to turn it off if the eigenvectors
  will not be used.
  Resolves josdejong#2180.
gwhitney added a commit to gwhitney/mathjs that referenced this issue Oct 5, 2023
  For large matrices, the eigenvector computation can be noticeably expensive
  and so it's worthwhile to have a way to turn it off if the eigenvectors
  will not be used.
  Resolves josdejong#2180.
@josdejong
Copy link
Owner

Fixed now in v12.0.0 via #3057.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants