-
-
Notifications
You must be signed in to change notification settings - Fork 598
Implement more algorithms for computing eigenvalues #39804
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
Conversation
785d796
to
6a308d0
Compare
Documentation preview for this PR (built with commit 74f8760; changes) is ready! 🎉 |
6a308d0
to
1724f23
Compare
[0 0 0 0 0] | ||
[0 0 0 0 0] | ||
sage: P*D-H*P == matrix.zero(5) | ||
True |
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.
This is necessary because now check=False
is passed, so exactify
is no longer called, so it shows some 0.000000?e-16
thing instead.
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.
LGTM modulo some details.
Co-authored-by: Travis Scrimshaw <[email protected]>
d9076dc
to
da1845a
Compare
da1845a
to
5a30e3c
Compare
I decide to make The reason being the functionality and interface never need to change, even if flint decide to remove |
Thought: is The situation is somewhat analogous to https://github.com/sagemath/sage/pull/39243/files where |
By benchmarking result it appears that |
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 that is good for the algorithm to not issue a warning.
I don't have an answer with regards to PARI. It's based on a potential future case, and we can worry about it later if that does happen.
Just one last little doc tweak, then a positive review.
Co-authored-by: Travis Scrimshaw <[email protected]>
Test failure looks like #22482 and unrelated to this one. (Still waiting for documentation to build to check it) Edit: the links work https://doc-pr-39804--sagemath.netlify.app/html/en/reference/matrices/sage/matrix/matrix2#hunk5 |
Thank you. Sorry, I thought that short version would work based on what is important at startup. Positive review. |
sagemathgh-39804: Implement more algorithms for computing eigenvalues Now `eigenvalues()` and `eigenvectors_left()` takes `algorithm` parameter. Partially handles sagemath#39762 Also changes the default algorithm of `eigenvalues()` to `pari_charpoly` when the base field is `RR` or `CC`. Additional changes required: * implement conversion from mpmath object to RR or CC * implement shifting elements of RR by a negative value * detect error of conversion to mpmath object * change a hard error to a warning ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [x] I have created tests covering the changes. - [x] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#39804 Reported by: user202729 Reviewer(s): Travis Scrimshaw, user202729
sagemathgh-39804: Implement more algorithms for computing eigenvalues Now `eigenvalues()` and `eigenvectors_left()` takes `algorithm` parameter. Partially handles sagemath#39762 Also changes the default algorithm of `eigenvalues()` to `pari_charpoly` when the base field is `RR` or `CC`. Additional changes required: * implement conversion from mpmath object to RR or CC * implement shifting elements of RR by a negative value * detect error of conversion to mpmath object * change a hard error to a warning ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [x] I have created tests covering the changes. - [x] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#39804 Reported by: user202729 Reviewer(s): Travis Scrimshaw, user202729
On 32-bit:
|
Are they all the failures? (I haven't figured how to install pari 32-bit on my machine) |
Yes, those were the only ones |
sagemathgh-39804: Implement more algorithms for computing eigenvalues Now `eigenvalues()` and `eigenvectors_left()` takes `algorithm` parameter. Partially handles sagemath#39762 Also changes the default algorithm of `eigenvalues()` to `pari_charpoly` when the base field is `RR` or `CC`. Additional changes required: * implement conversion from mpmath object to RR or CC * implement shifting elements of RR by a negative value * detect error of conversion to mpmath object * change a hard error to a warning ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [x] I have created tests covering the changes. - [x] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#39804 Reported by: user202729 Reviewer(s): Travis Scrimshaw, user202729
sagemathgh-39804: Implement more algorithms for computing eigenvalues Now `eigenvalues()` and `eigenvectors_left()` takes `algorithm` parameter. Partially handles sagemath#39762 Also changes the default algorithm of `eigenvalues()` to `pari_charpoly` when the base field is `RR` or `CC`. Additional changes required: * implement conversion from mpmath object to RR or CC * implement shifting elements of RR by a negative value * detect error of conversion to mpmath object * change a hard error to a warning ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [x] I have created tests covering the changes. - [x] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#39804 Reported by: user202729 Reviewer(s): Travis Scrimshaw, user202729
sagemathgh-39804: Implement more algorithms for computing eigenvalues Now `eigenvalues()` and `eigenvectors_left()` takes `algorithm` parameter. Partially handles sagemath#39762 Also changes the default algorithm of `eigenvalues()` to `pari_charpoly` when the base field is `RR` or `CC`. Additional changes required: * implement conversion from mpmath object to RR or CC * implement shifting elements of RR by a negative value * detect error of conversion to mpmath object * change a hard error to a warning ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [x] I have created tests covering the changes. - [x] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#39804 Reported by: user202729 Reviewer(s): Travis Scrimshaw, user202729
Now
eigenvalues()
andeigenvectors_left()
takesalgorithm
parameter.Partially handles #39762
Also changes the default algorithm of
eigenvalues()
topari_charpoly
when the base field isRR
orCC
.Additional changes required:
📝 Checklist
⌛ Dependencies