Opened 4 years ago

Last modified 2 years ago

#16115 new enhancement

Class misconception with isogenies

Reported by: sbesnier Owned by:
Priority: major Milestone: sage-6.4
Component: elliptic curves Keywords: days57
Cc: defeo Merged in:
Authors: Sébastien Besnier Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #12880 Stopgaps:

Description

The isogenies implementation suffers several conception issues; the main issues I can quickly describe are:

  • the _mul_ method of EllipticCurveIsogeny? is not handled by the general Morphism class:
    sage: E = EllipticCurve(j=GF(7)(0))
    sage: phi=E.isogeny( [E(0), E((0,1)), E((0,-1))])
    sage: phi*phi
    [...]
    TypeError: Elliptic Curve defined by y^2 = x^3 + 1 over Finite Field of size 7 is not in Category of hom sets in Category of Schemes
    

--> I've edited ell_curve_isogeny.py in order to make this work: the codomain() and domain() methods initially gaves "Elliptic Curve ...", I've updated to "Abelian group of points of Elliptic Curve ..." (and tried to make the other parts of code consistant with that).

Follwing the previous code:

sage: i=WeierstrassIsomorphism(E,(2,0,0,0))
sage: phi*i
Composite map:
  From: Abelian group of points on Elliptic Curve defined by y^2 = x^3 + 1 over Finite Field of size 7
  To:   Abelian group of points on Elliptic Curve defined by y^2 = x^3 + 1 over Finite Field of size 7
  Defn:   Generic morphism:
          From: Abelian group of points on Elliptic Curve defined by y^2 = x^3 + 1 over Finite Field of size 7
          To:   Abelian group of points on Elliptic Curve defined by y^2 = x^3 + 1 over Finite Field of size 7
          Via:  (u,r,s,t) = (2, 0, 0, 0)
        then
          Isogeny of degree 3:
          From: Abelian group of points on Elliptic Curve defined by y^2 = x^3 + 1 over Finite Field of size 7
          To: Abelian group of points on Elliptic Curve defined by y^2 = x^3 + 1 over Finite Field of size 7
sage: i*phi
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-13-b2d0bfcb3b82> in <module>()
----> 1 i*phi

/home/sebsheep/sage/local/lib/python2.7/site-packages/sage/schemes/elliptic_curves/weierstrass_morphism.pyc in __mul__(self, other)
    582             True
    583         """
--> 584         if self._domain_curve==other._codomain_curve:
    585             w=baseWI.__mul__(self,other)
    586             return WeierstrassIsomorphism(other._domain_curve, w.tuple(), self._codomain_curve)

/home/sebsheep/sage/local/lib/python2.7/site-packages/sage/structure/element.so in sage.structure.element.Element.__getattr__ (sage/structure/element.c:3868)()

/home/sebsheep/sage/local/lib/python2.7/site-packages/sage/structure/misc.so in sage.structure.misc.getattr_from_other_class (sage/structure/misc.c:1696)()

AttributeError: 'EllipticCurveIsogeny' object has no attribute '_codomain_curve'

I've joined a "preliminary draft" of a new "sage/local/lib/python2.7/site-packages/sage/schemes/elliptic_curves/ell_curve_isogeny.py" wrote in a few hours.

Attachments (1)

ell_curve_isogeny.py (138.3 KB) - added by sbesnier 4 years ago.
first draft

Download all attachments as: .zip

Change History (11)

Changed 4 years ago by sbesnier

first draft

comment:1 Changed 4 years ago by pbruin

  • Component changed from algebraic geometry to elliptic curves
  • Dependencies set to #12880

There are indeed various aspects in which isogenies could be improved.

As for your first point, this should be addressed in #12880. (Note that the mathematically correct solution is to make the domain and codomain be the elliptic curves themselves, not the groups of points.) This can only really be fixed after #11474.

Tying the various classes closer together as in your second point is certainly worth looking at. One could even consider making EllipticCurveIsogeny inherit from SchemeMorphism_polynomial_projective_space, but perhaps that class is too specialised in an orthogonal direction.

It is not recommended to simply upload the Python files you changed; in that way it is impossible to see what your changes are or on what version of Sage you based them. Use Git instead; see http://sagemath.org/doc/developer/ for an introduction to the Sage development process.

Finally, please be aware that "misconception" is not the most charitable way to describe the work that has been done so far. A lot of code in Sage dates from years back when the "right" infrastructure (more specific base classes, category code etc.) was simply not available. There is certainly a lot of room to improve the design in many parts of Sage, but time and resources are limited, and changes need to be made in small steps to avoid breaking existing code.

comment:2 Changed 4 years ago by pbruin

See also #7368 for another to-do list for isogenies.

comment:3 Changed 4 years ago by defeo

  • Cc defeo added

comment:4 Changed 4 years ago by defeo

  • Keywords days57 added

comment:5 Changed 4 years ago by defeo

  • Authors changed from sbesnier to Sébastien Besnier

comment:6 Changed 4 years ago by jpflori

About SchemeMorphism also see the description of #14711 of why it currently does not inherit Morphism (simple enough reason, cython won't let you do that currently...).

IIRC this was the corresponding discussion:

comment:7 Changed 4 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:8 Changed 3 years ago by cremona

I implemented Weierstrass morphisms in 2007: it was the first piece of Sage development I did. Until that time it was impossible to change models or move points from one model to another! And the implementation of isogenies was done much later than that.

I do agree that the EllipticCurveIsogeny? class is not designed in the way I would want it to be, but it works well enough and has been used an enormous amount over QQ and over number fields, even if it has not been possible (for example) to compose isogenies.

I also agree with Peter that the domain and codomain must be elliptic curves, and not sets or groups of points.

I am currently working on extending the class IsogenyClass_EC to work over number fields (see #16743). I don't expect that the work done on this and related tickets will affect that a lot.

Last edited 3 years ago by cremona (previous) (diff)

comment:9 Changed 3 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:10 Changed 2 years ago by adhalanay

added

Note: See TracTickets for help on using tickets.