Opened 9 years ago
Closed 9 years ago
#12766 closed enhancement (fixed)
Better plotting for elliptic curves
Reported by: | roed | Owned by: | cremona |
---|---|---|---|
Priority: | minor | Milestone: | sage-5.0 |
Component: | elliptic curves | Keywords: | |
Cc: | wstein, cremona, kedlaya | Merged in: | sage-5.0.beta14 |
Authors: | David Roe | Reviewers: | Kiran Kedlaya |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
The plot for EllipticCurve('448c6')
looks like a vertical line. The reason is that plot
contains
d = 4*x**3 + (a1**2 + 4*a2)*x**2 + (2*a3*a1 + 4*a4)*x + (a3**2 + 4*a6) r = d.roots(multiplicities=False) r.sort() if xmax is None: xmax = r[-1] + 2 xmax = max(xmax, r[-1]+2) if xmin is None: xmin = r[0] - 2 xmin = min(xmin, r[0]-2)
When 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.
Attachments (2)
Change History (9)
comment:1 Changed 9 years ago by
- Description modified (diff)
Changed 9 years ago by
comment:2 Changed 9 years ago by
- Cc wstein cremona added
- Status changed from new to needs_review
comment:3 Changed 9 years ago by
- Cc kedlaya added
Changed 9 years ago by
comment:4 Changed 9 years ago by
- Reviewers set to Kiran Kedlaya
- Status changed from needs_review to positive_review
Patchbot says all tests pass when applied against 5.0beta3, and looks good otherwise. Positive review.
comment:5 Changed 9 years ago by
I'm assuming both patches have to be applied...?
comment:6 Changed 9 years ago by
Correct.
comment:7 Changed 9 years ago by
- Merged in set to sage-5.0.beta14
- Resolution set to fixed
- Status changed from positive_review to closed
Note: See
TracTickets for help on using
tickets.
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....