Opened 9 years ago

Closed 9 years ago

#12166 closed defect (fixed)

Elliptic curves: improve documentation of period_lattice

Reported by: johanbosman Owned by: cremona
Priority: minor Milestone: sage-4.8
Component: elliptic curves Keywords:
Cc: Merged in: sage-4.8.alpha5
Authors: Johan Bosman Reviewers: John Cremona
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

sage: E = EllipticCurve([1,2,3,4,5])
sage: E.period_lattice?
...
Docstring:
       Returns the period lattice of the elliptic curve.
    
       INPUT:
    
       * "embedding" - ignored (for compatibility with the period_lattice
         function for elliptic_curve_number_field)
    
       OUTPUT:
    
       (period lattice) The PeriodLattice_ell object associated to this
       elliptic curve (with respect to the natural embedding of QQ into
       RR).
...

The documentation speaks of "the" period lattice of E, but to define a period lattice one needs an invariant differential as well. From the documentation, it is not clear to me which differential is chosen here.

Attachments (1)

12166.patch (1.4 KB) - added by johanbosman 9 years ago.

Download all attachments as: .zip

Change History (6)

comment:1 follow-up: Changed 9 years ago by cremona

  • Summary changed from Elliptic curves: improve documentation of documentation of period_lattice to Elliptic curves: improve documentation of period_lattice

How pedantic you are! Of course it means dx/(2y+a1*x+a3), what else? But feel free to change that (just as I have corrected the title of this ticket). Don't forget to also look at the function for elliptic curves over number fields (where there is a lot about choosing the real or complex embedding).

comment:2 in reply to: ↑ 1 Changed 9 years ago by johanbosman

Replying to cremona:

How pedantic you are! Of course it means dx/(2y+a1*x+a3), what else?

It could be either that or a Néron differential, or perhaps people use other normalizations. I really didn't know which one was chosen or what is considered to be "standard" here. Thanks for clarifying!

Changed 9 years ago by johanbosman

comment:3 Changed 9 years ago by johanbosman

  • Status changed from new to needs_review

comment:4 Changed 9 years ago by cremona

  • Authors set to Johan Bosman
  • Reviewers set to John Cremona
  • Status changed from needs_review to positive_review

Looks good! (Applies fine to 4.8.alpha4)

comment:5 Changed 9 years ago by jdemeyer

  • Merged in set to sage-4.8.alpha5
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.