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
Better plotting for elliptic curves #12766
Comments
This comment has been minimized.
This comment has been minimized.
Attachment: 12766.patch.gz |
Author: David Roe |
comment:2
Ready for review. You can find a worksheet for testing at http://sagenb.org/home/pub/4639. It's all using the old code, but you can see some of the problems. Compare with the new code by downloading your own copy and running locally. This would be nicer if we had branches on sagenb already.... |
Attachment: 12766_2.patch.gz |
Reviewer: Kiran Kedlaya |
comment:4
Patchbot says all tests pass when applied against 5.0beta3, and looks good otherwise. Positive review. |
comment:5
I'm assuming both patches have to be applied...? |
comment:6
Correct. |
Merged: sage-5.0.beta14 |
The plot for
EllipticCurve('448c6')
looks like a vertical line. The reason is thatplot
containsWhen
d
has only one real root, this approach doesn't work that well. One suggestion would be to also require that the plot contains the flex points, which can be found from the 3-division polynomial.CC: @williamstein @JohnCremona @kedlaya
Component: elliptic curves
Author: David Roe
Reviewer: Kiran Kedlaya
Merged: sage-5.0.beta14
Issue created by migration from https://trac.sagemath.org/ticket/12766
The text was updated successfully, but these errors were encountered: