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

Custom SortRule #147

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Custom SortRule #147

wants to merge 5 commits into from

Conversation

toonijn
Copy link

@toonijn toonijn commented Dec 7, 2022

This pull request provides the ability to use user-defined selection criteria, without breaking API usage.

Upon using Spectra, I was very impressed by its usability but was missing a way to add a custom eigenvalue selection rule. In my case I wanted to select the smallest non-zero eigenvalues. Adding this functionality to Spectra was a bit more involved than expected. As such, this pull request is quite large.

To make user defined selection criteria work the EigenvalueSorter has now a std::function member which it uses as a sort target. To avoid fundamentally breaking API compatibility. Values of the enum SortRule are implicitly cast to an appropriate EigenvalueSorter.

One of the drawbacks of this approach is that we now rely upon the compiler to optimize many calls to this std::function. In my opinion, this is not a big issue because the number of eigenvalues always will be relatively small. Also, the overhead of O(n log(n)) function calls is negligible with respect to the O(n³) QR-decomposition in each step. Alternatively, if you find this overhead unacceptable, I may be able to rewrite it with some more templates. Such that argsort becomes a virtual function where the target-function is a template parameter and can be inlined. This would reduce the virtual function calls from n log(n) to 1, with the cost of much more complex code base, and a less readable implementation.

How to use a user-defined selection criterium:

GenEigsSolver<...> eigsSolver(...);
eigsSolver.init();
EigenvalueSorter<std::complex<double>> closestToTarget{[](std::complex<double> eigenvalue) {
    return std::abs(eigenvalue - 10.);
}};
eigsSolver.compute(closestToTarget);

@shivupa
Copy link
Contributor

shivupa commented Feb 17, 2023

Hi I'm not a maintainer, and I do think this flexibility is nice. However for your case couldn't you use Spectra::GenEigsRealShiftSolver with $\sigma=0.0$ combined with the Spectra::SortRule::SmallestMagn?

@toonijn
Copy link
Author

toonijn commented Feb 20, 2023

Maybe I am mistaken, but I think SmallestMagn also selects the zero eigenvalues. My custom sortrule is something like:

[](const std::complex<Scalar> &r) {
    if (r.real() < 1e-5 || abs(r.imag()) > 1e-5)
        return Eigen::NumTraits<Scalar>::infinity();
    return r.real();
}

which selects the smallest real eigenvalues which are larger than 1e-5.

@shivupa
Copy link
Contributor

shivupa commented Feb 21, 2023

Oh I understand now. You are right.

@yixuan
Copy link
Owner

yixuan commented Feb 22, 2023

Hi @toonijn, I apologize that at this moment I don't have time to look at the PR carefully, but I expect this would be a nice feature. I'll handle it once I get some free time. Thanks.

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.

None yet

3 participants