Custom SortRule#147
Open
toonijn wants to merge 5 commits into
Open
Conversation
Contributor
|
Hi I'm not a maintainer, and I do think this flexibility is nice. However for your case couldn't you use |
Author
|
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. |
Contributor
|
Oh I understand now. You are right. |
Owner
|
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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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: