-
Notifications
You must be signed in to change notification settings - Fork 114
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
[algorithms.eigs] add implicitly restarted arnoldi method #880
Conversation
I still want to try it on some benchmarks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tried eigs
on a few SLICOT benchmarks and it works as good as I would have expected, i.e., very good for eigenvalues far from zero and not so good otherwise.
Below are some comments just from looking at the docstring.
Similar to the samdp
module, it might be good to add some isinstance
assertions in the eigs
function and make other functions private (also, it would be good to write QR_iteration
in lowercase).
I made the changes and also added myself to AUTHORS.md. For eigenvalues that are close to zero I will probably add a shift and invert option sometime in the near future. |
This'll need another rebase on master after #922 got merged for some CI fixes. Again. Sorry. |
Codecov Report
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @lbalicki, here are a few more comments. I would be for merging after resolving these.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did not check the algorithm itself. Apart from a few minor comments, everything looks great!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can merge this now ..
Agreed. Thanks @lbalicki. |
I implemented the implicitly restarted arnoldi method for computing a few eigenvalues of a real
Operator
. This is related to the issues #774 and #772.