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

Merge DFTK's implementation of LOBPCG into IterativeSolvers #329

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

GVigne
Copy link

@GVigne GVigne commented Oct 21, 2022

Hi!
Since the first time we discussed about the implementation of the LOBPCG algorithm (#246), we (and by that I mean @mfherbst and @antoine-levitt) have made some improvements on the algorithm currently used in the DFTK package. Currently, it is faster, more stable and a bit more readable (so hopefully, easy to maintain) then the current implementation in IterativeSolvers. We thought it would be nice to move our implementation of LOBPCG to IterativeSolvers, and this is the goal of this PR.

This implementation is based on Hetmaniuk and Lehoucq's paper "Basis selection in LOBPCG" (https://doi.org/10.1016/j.jcp.2006.02.007), but using double Cholesky factorizations to orthogonalize vectors (see https://arxiv.org/pdf/1809.11085.pdf) as it is fast and can be easily used in parallel computing.

This implementation is also GPU compatible: one can simply pass A and X as GPU arrays and the resulting eigenvectors and eigenvalues will also be GPU arrays.

There is probably a lot to talk about, and some more things to implement, like allowing the user to create and use his own callback method.

@mschauer
Copy link
Member

There is probably a lot to talk about, and some more things to implement, like allowing the user to create and use his own callback method.

Is anything of that blocking for this PR?

@GVigne
Copy link
Author

GVigne commented Oct 21, 2022

The reason tests are failing is that I removed some of the keywords arguments as either they weren't useful anymore or because I thought we needed to see how they should be implemented. That's the case for example for log and not_zeros: the tests are failing because lobpcg gets unsupported keyword arguments.

@GVigne
Copy link
Author

GVigne commented Nov 2, 2022

@mohamed82008 any ideas/reactions/remarks?

@mschauer
Copy link
Member

mschauer commented Nov 2, 2022

@platawiec, @GVigne, @mohamed82008 because this package is so much unmaintained, one way forward I would see is that you mutually review your pull request. Does this make sense? I am only here as a general math person and because of the interest in the IDR(s) method, but I am not a proper maintainer of this package.

@mohamed82008
Copy link
Member

I can review this some time next week if that's not too late.

@GVigne
Copy link
Author

GVigne commented Nov 3, 2022

It's not too late for me: as I said, there's probably a lot to discuss, so it might take some time anyways. I just wanted to have some insights to get things moving.

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

Successfully merging this pull request may close these issues.

3 participants