Ticket #9371 (closed task: fixed)
Implement E.two_torsion_rank() over number fields
| Reported by: | weigandt | Owned by: | weigandt |
|---|---|---|---|
| Priority: | major | Milestone: | sage-4.7 |
| Component: | elliptic curves | Keywords: | elliptic curves, two torsion rank |
| Cc: | cremona | Work issues: | |
| Report Upstream: | N/A | Reviewers: | John Cremona, Gagan Sekhon |
| Authors: | Jamie Weigandt, Aly Deines | Merged in: | sage-4.7.alpha5 |
| Dependencies: | Stopgaps: |
Description
The function E.two_torsion_rank() can easily be made to work over number fields. The current implementation over QQ calls E.torsion_subgroup() and makes nontrivial use of Mazur's torsion theorem. This should be more efficient and more general by considering the 2-division polynomial.
Attachments
-
trac_9371_two_torsion_rank.patch
(4.8 KB) -
added by weigandt 3 years ago.
- Extend E.two_torsion_rank() method to number fields. Applies to 4.4.4
-
trac_9371_field_two_torsion.patch
(2.8 KB) -
added by weigandt 2 years ago.
- new patch, replaced old one, applies to 4.6.2
-
trac_9371_field_two_torsion.2.patch
(3.2 KB) -
added by aly.deines 2 years ago.
- added documentation, replaces previous patch
Change History
Changed 3 years ago by weigandt
-
attachment
trac_9371_two_torsion_rank.patch
added
Extend E.two_torsion_rank() method to number fields. Applies to 4.4.4
comment:3 Changed 3 years ago by cremona
- Status changed from needs_review to needs_work
- Reviewers set to John Cremona
Looks good: a better method and more general. However: why not move the function all the way up to ell_field? There's no reason at all why the same code would not work over any field of char. not 2, and even in char. 2 (where the result is at most 0 or 1 for supersingular/ordinary curves, but so what).
If you do that, add extra doctests over (say) finite fields. While you are at it, one thing about the docstring could be improved: the short description should fit on one line, so cut it after E(K), and put the rest into a separate ALGORITHM block. "Needs work" sounds negative, so let me elaborate: this is good and with a tiny amount of work would be very good!
Changed 2 years ago by weigandt
-
attachment
trac_9371_field_two_torsion.patch
added
new patch, replaced old one, applies to 4.6.2
comment:5 Changed 2 years ago by gagansekhon
I think there should be at least 2 more different examples like you had before. Either you can add then I can review or I can add and we will need to find a new reviewer
Changed 2 years ago by aly.deines
-
attachment
trac_9371_field_two_torsion.2.patch
added
added documentation, replaces previous patch
comment:6 Changed 2 years ago by gagansekhon
- Status changed from needs_review to positive_review
- Reviewers changed from John Cremona to John Cremona, Gagan Sekhon
- Authors changed from Jamie Weigandt to Jamie Weigandt, Aly.deines
it initially failed sage/interface/r.py, but once I ran it separately and it worked.
