Sage: Ticket #16115: Class misconception with isogenies
https://trac.sagemath.org/ticket/16115
<p>
The isogenies implementation suffers several conception issues; the main issues I can quickly describe are:
</p>
<ul><li>the _mul_ method of <a class="missing wiki">EllipticCurveIsogeny?</a> is not handled by the general Morphism class:
<pre class="wiki">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
</pre></li></ul><p>
--> 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).
</p>
<p>
</p>
<ul><li>Hierarchy class is not well think: e.g. <a class="missing wiki">WeierstrassIsomorphism?</a> is not a child of <a class="missing wiki">EllipticCurveIsogeny?</a>. Moreover, we can not compose isogeny and Weierstrass Isomorphism. There exist a "<a class="missing wiki">SchemeMorphism?</a>" class; maybe it's a good idea to make <a class="missing wiki">EllipticCurveIsogeny?</a> subclass of this ?
</li></ul><p>
Follwing the previous code:
</p>
<pre class="wiki">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'
</pre><p>
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.
</p>
en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/16115
Trac 1.1.6sbesnierWed, 09 Apr 2014 20:36:07 GMTattachment set
https://trac.sagemath.org/ticket/16115
https://trac.sagemath.org/ticket/16115
<ul>
<li><strong>attachment</strong>
set to <em>ell_curve_isogeny.py</em>
</li>
</ul>
<p>
first draft
</p>
TicketpbruinThu, 10 Apr 2014 18:32:12 GMTcomponent changed; dependencies set
https://trac.sagemath.org/ticket/16115#comment:1
https://trac.sagemath.org/ticket/16115#comment:1
<ul>
<li><strong>dependencies</strong>
set to <em>#12880</em>
</li>
<li><strong>component</strong>
changed from <em>algebraic geometry</em> to <em>elliptic curves</em>
</li>
</ul>
<p>
There are indeed various aspects in which isogenies could be improved.
</p>
<p>
As for your first point, this should be addressed in <a class="closed ticket" href="https://trac.sagemath.org/ticket/12880" title="defect: Inconsistent domain, codomain and parent in EllipticCurveIsogeny (closed: fixed)">#12880</a>. (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 <a class="closed ticket" href="https://trac.sagemath.org/ticket/11474" title="defect: Elliptic curves should be unique parent structures (closed: fixed)">#11474</a>.
</p>
<p>
Tying the various classes closer together as in your second point is certainly worth looking at. One could even consider making <code>EllipticCurveIsogeny</code> inherit from <code>SchemeMorphism_polynomial_projective_space</code>, but perhaps that class is too specialised in an orthogonal direction.
</p>
<p>
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 <a class="ext-link" href="http://sagemath.org/doc/developer/"><span class="icon"></span>http://sagemath.org/doc/developer/</a> for an introduction to the Sage development process.
</p>
<p>
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.
</p>
TicketpbruinFri, 11 Apr 2014 11:16:32 GMT
https://trac.sagemath.org/ticket/16115#comment:2
https://trac.sagemath.org/ticket/16115#comment:2
<p>
See also <a class="new ticket" href="https://trac.sagemath.org/ticket/7368" title="enhancement: Further work on isogenies and endomorphisms of elliptic curves (new)">#7368</a> for another to-do list for isogenies.
</p>
TicketdefeoFri, 11 Apr 2014 22:00:55 GMTcc set
https://trac.sagemath.org/ticket/16115#comment:3
https://trac.sagemath.org/ticket/16115#comment:3
<ul>
<li><strong>cc</strong>
<em>defeo</em> added
</li>
</ul>
TicketdefeoFri, 11 Apr 2014 22:03:04 GMTkeywords set
https://trac.sagemath.org/ticket/16115#comment:4
https://trac.sagemath.org/ticket/16115#comment:4
<ul>
<li><strong>keywords</strong>
<em>days57</em> added
</li>
</ul>
TicketdefeoFri, 11 Apr 2014 22:05:19 GMTauthor changed
https://trac.sagemath.org/ticket/16115#comment:5
https://trac.sagemath.org/ticket/16115#comment:5
<ul>
<li><strong>author</strong>
changed from <em>sbesnier</em> to <em>Sébastien Besnier</em>
</li>
</ul>
TicketjpfloriFri, 18 Apr 2014 15:49:34 GMT
https://trac.sagemath.org/ticket/16115#comment:6
https://trac.sagemath.org/ticket/16115#comment:6
<p>
About <code>SchemeMorphism</code> also see the description of <a class="closed ticket" href="https://trac.sagemath.org/ticket/14711" title="defect: Weak references in the coercion graph (closed: fixed)">#14711</a> of why it currently does not inherit <code>Morphism</code> (simple enough reason, cython won't let you do that currently...).
</p>
<p>
IIRC this was the corresponding discussion:
</p>
<ul><li><a class="ext-link" href="https://groups.google.com/d/msg/cython-users/-GvwK6sEF0E/vAaliNBQ9XMJ"><span class="icon"></span>https://groups.google.com/d/msg/cython-users/-GvwK6sEF0E/vAaliNBQ9XMJ</a>
</li></ul>
Ticketvbraun_spamTue, 06 May 2014 15:20:58 GMTmilestone changed
https://trac.sagemath.org/ticket/16115#comment:7
https://trac.sagemath.org/ticket/16115#comment:7
<ul>
<li><strong>milestone</strong>
changed from <em>sage-6.2</em> to <em>sage-6.3</em>
</li>
</ul>
TicketcremonaWed, 30 Jul 2014 13:12:10 GMT
https://trac.sagemath.org/ticket/16115#comment:8
https://trac.sagemath.org/ticket/16115#comment:8
<p>
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.
</p>
<p>
I do agree that the <a class="missing wiki">EllipticCurveIsogeny?</a> 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.
</p>
<p>
I also agree with Peter that the domain and codomain must be elliptic curves, and not sets or groups of points.
</p>
<p>
I am currently working on extending the class IsogenyClass_EC to work over number fields (see <a class="closed ticket" href="https://trac.sagemath.org/ticket/16743" title="enhancement: Extend IsogenyClass_EC to work over number fields (closed: fixed)">#16743</a>). I don't expect that the work done on this and related tickets will affect that a lot.
</p>
Ticketvbraun_spamSun, 10 Aug 2014 16:51:03 GMTmilestone changed
https://trac.sagemath.org/ticket/16115#comment:9
https://trac.sagemath.org/ticket/16115#comment:9
<ul>
<li><strong>milestone</strong>
changed from <em>sage-6.3</em> to <em>sage-6.4</em>
</li>
</ul>
TicketadhalanayTue, 20 Oct 2015 18:45:44 GMT
https://trac.sagemath.org/ticket/16115#comment:10
https://trac.sagemath.org/ticket/16115#comment:10
<p>
added
</p>
Ticket