Opened 11 years ago

Closed 11 years ago

#4290 closed defect (fixed)

[with patch, positive review] function to construct an Elliptic Curve from a plane curve of genus one (using Magma)

Reported by: ljpk Owned by: craigcitro
Priority: minor Milestone: sage-4.1
Component: algebraic geometry Keywords:
Cc: Merged in: sage-4.1.alpha0
Authors: Lloyd Kilford, John Cremona Reviewers: Nick Alexander
Report Upstream: Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description

This is a function to turn plane curves of genus 1 with a known point into objects of type EllipticCurve?. It relies on MAGMA (as does EllipticCurve_from_cubic) but will add a little functionality. Here is an example of how it works:

sage: x,y,z=MPolynomialRing(QQ,Integer(3),'xyz').gens() # optional
sage: C=Curve(x^4+x^2*y^2-z^4) 
sage: P=C(1,0,1) 
sage: E=EllipticCurve_from_plane_curve(C,P) 
sage: E 
Elliptic Curve defined by y^2  = x^3 + 4*x over Rational Field

Attachments (3)

EllipticCurve_from_plane_curve.sage (1.5 KB) - added by ljpk 11 years ago.
implementation of EllipticCurve_from_plane_curve
trac_4290.patch (3.8 KB) - added by cremona 11 years ago.
Patch based on the above, based on 3.4.1.rc0
trac_4290-rebase.patch (6.2 KB) - added by cremona 11 years ago.
Replaces previous; rebased to 4.0.1

Download all attachments as: .zip

Change History (13)

Changed 11 years ago by ljpk

implementation of EllipticCurve_from_plane_curve

comment:1 Changed 11 years ago by ljpk

  • Owner changed from was to craigcitro

comment:2 Changed 11 years ago by was

  • Summary changed from stopgap function to turn plane curves of genus one into elliptic curves to [with patch; needs review] stopgap function to turn plane curves of genus one into elliptic curves

comment:3 Changed 11 years ago by cremona

  • Summary changed from [with patch; needs review] stopgap function to turn plane curves of genus one into elliptic curves to [with patch; with review, needs work] stopgap function to turn plane curves of genus one into elliptic curves

The "patch" isn't a patch, it's just a function defined in Sage. Secondly, there is already a function that does almost the same as this in sage/schemes/elliptic_curves/constructor.py .

So what needs to be done (Lloyd!) is to (1) Explain briefly how this function differs from the existing one, and why it is better, and also (2) actually make a patch.

If Lloyd convinces me of (1) then I'll happily do (2).

comment:4 Changed 11 years ago by ljpk

The current Sage function only converts cubics into elliptic curves, but MAGMA can do quite a bit more than that. My function can also cope with other genus one curves such as x4+x2*y2-z4 which is not cubic. Possibly the current function should be tweaked slightly so that it can do both cubic and non-cubic examples.

comment:5 Changed 11 years ago by mabshoff

  • Summary changed from [with patch; with review, needs work] stopgap function to turn plane curves of genus one into elliptic curves to [with patch; needs work] stopgap function to turn plane curves of genus one into elliptic curves

Anything going on with this patch? It has been sitting around for 4 months :(.

Cheers,

Michael

Changed 11 years ago by cremona

Patch based on the above, based on 3.4.1.rc0

comment:6 Changed 11 years ago by cremona

  • Summary changed from [with patch; needs work] stopgap function to turn plane curves of genus one into elliptic curves to [with new patch; needs review] function to construct an Elliptic Curve from a plane curve of genus one (using Magma)

I have made a proper patch based on Lloyd's function (based on 3.4.1.rc0), which also fixes his function so that it works when the variable names are something other than 'x','y','z' (which it did not before).

You might think that this function makes the existing EllipicCurve_from_cubic redundant; it nearly does, but that function takes a homogeneous polynomial while this one takes an actual curve. It would probably be better to combine these when all this is rewritten in Sage.

I added the new function to all.py so that it is in the global namespace. I am not sure what the convention is here, given that it is an optional-only function requiring magma! Also I have nto tested my patch o na machine without magma since I don't have one handy :)

comment:7 Changed 11 years ago by was

  • Summary changed from [with new patch; needs review] function to construct an Elliptic Curve from a plane curve of genus one (using Magma) to [with new patch; needs work] function to construct an Elliptic Curve from a plane curve of genus one (using Magma)

The doctests should be marked:

   # optional - magma

so when one does

  sage -t -only_optional=magma

they get run.

Changed 11 years ago by cremona

Replaces previous; rebased to 4.0.1

comment:8 Changed 11 years ago by cremona

  • Summary changed from [with new patch; needs work] function to construct an Elliptic Curve from a plane curve of genus one (using Magma) to [with new patch; needs review] function to construct an Elliptic Curve from a plane curve of genus one (using Magma)

The new patch replaces previous ones. It marks the doctests as William asked; testing then revealed bugs which have been fixed. Also rebased to 4.0.1 and the docstrings properly ReSTified to the reference manual page looks good.

comment:9 Changed 11 years ago by ncalexan

  • Authors set to Lloyd Kilford, John Cremona
  • Reviewers set to Nick Alexander
  • Summary changed from [with new patch; needs review] function to construct an Elliptic Curve from a plane curve of genus one (using Magma) to [with patch, positive review] function to construct an Elliptic Curve from a plane curve of genus one (using Magma)

Works well for me. Pity it requires magma :(

comment:10 Changed 11 years ago by rlm

  • Merged in set to sage-4.1.alpha0
  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.