Opened 11 years ago

Closed 10 years ago

Last modified 10 years ago

#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 Merged in: sage-4.7.alpha5
Authors: Jamie Weigandt, Aly Deines Reviewers: John Cremona, Gagan Sekhon
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

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 (3)

trac_9371_two_torsion_rank.patch (4.8 KB) - added by weigandt 11 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 10 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 10 years ago.
added documentation, replaces previous patch

Download all attachments as: .zip

Change History (11)

comment:1 Changed 11 years ago by weigandt

  • Authors changed from weigandt to Jamie Weigandt

Changed 11 years ago by weigandt

Extend E.two_torsion_rank() method to number fields. Applies to 4.4.4

comment:2 Changed 11 years ago by weigandt

  • Status changed from new to needs_review

comment:3 Changed 11 years ago by cremona

  • Reviewers set to John Cremona
  • Status changed from needs_review to needs_work

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 10 years ago by weigandt

new patch, replaced old one, applies to 4.6.2

comment:4 Changed 10 years ago by weigandt

  • Status changed from needs_work to needs_review

comment:5 Changed 10 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 10 years ago by aly.deines

added documentation, replaces previous patch

comment:6 Changed 10 years ago by gagansekhon

  • Authors changed from Jamie Weigandt to Jamie Weigandt, Aly.deines
  • Reviewers changed from John Cremona to John Cremona, Gagan Sekhon
  • Status changed from needs_review to positive_review

it initially failed sage/interface/r.py, but once I ran it separately and it worked.

comment:7 Changed 10 years ago by jdemeyer

  • Merged in set to sage-4.7.alpha5
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:8 Changed 10 years ago by jdemeyer

  • Authors changed from Jamie Weigandt, Aly.deines to Jamie Weigandt, Aly Deines
Note: See TracTickets for help on using tickets.