Sage: Ticket #11474: Elliptic curves should be unique parent structures
https://trac.sagemath.org/ticket/11474
<p>
While elliptic curves are derived from <code>sage.structure.parent.Parent</code>, they violate the "unique parent" condition:
</p>
<pre class="wiki">sage: K = GF(1<<50,'t')
sage: j = K.random_element()
sage: from sage.structure.parent import Parent
sage: isinstance(EllipticCurve(j=j),Parent)
True
sage: EllipticCurve(j=j) is EllipticCurve(j=j)
False
sage: EllipticCurve(j=j) == EllipticCurve(j=j)
True
</pre><p>
Some people on <a class="ext-link" href="http://groups.google.com/group/sage-nt/browse_thread/thread/ec8d0ad14a819082"><span class="icon"></span>sage-nt</a> originally agreed that it is a bug. Other people think it isn't...
</p>
en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/11474
Trac 1.1.6SimonKingTue, 14 Jun 2011 09:01:10 GMT
https://trac.sagemath.org/ticket/11474#comment:1
https://trac.sagemath.org/ticket/11474#comment:1
<p>
My approach is:
</p>
<p>
A) Let <code>EllipticCurve_generic</code> inherit from <code>UniqueRepresentation</code>. If I am not mistaken, every other elliptic curve inherits from that, so, that should be fine.
</p>
<p>
B) The <code>__init__</code> methods should be uniform: <em>All</em> <code>__init__</code> methods should accept precisely one argument, namely an immutable sequence "ainvs" (in particular, the underlying field can be obtained from ainvs).
</p>
<p>
C) By <code>__classcall__</code> methods, make sure that the existing ways of constructing an elliptic curve will still work. In particular, it will create the immutable sequence "ainv".
</p>
<p>
One detail to consider: Sometimes an elliptic curve is taken from the Cremona database (see <code>sage.schemes.elliptic_curves.ell_rational_field</code>). The database provides certain attributes. It is possible that an elliptic curve <em>with the same a-invariant</em> is already in the cache, ignorant of the additional attributes.
</p>
<p>
But the classcall method could very well assign those additional attributes to an elliptic curve found in the cache before returning it. So, it should still work.
</p>
TicketSimonKingTue, 14 Jun 2011 09:04:40 GMT
https://trac.sagemath.org/ticket/11474#comment:2
https://trac.sagemath.org/ticket/11474#comment:2
<p>
Or should perhaps <code>ProjectiveCurve_generic</code> already be a unique parent? That's what elliptic curves derive from. Or even <code>AlgebraicScheme_subscheme</code>? How far should one go?
</p>
TicketSimonKingWed, 15 Jun 2011 10:46:44 GMTdescription changed
https://trac.sagemath.org/ticket/11474#comment:3
https://trac.sagemath.org/ticket/11474#comment:3
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/11474?action=diff&version=3">diff</a>)
</li>
</ul>
TicketSimonKingWed, 15 Jun 2011 13:22:27 GMTattachment set
https://trac.sagemath.org/ticket/11474
https://trac.sagemath.org/ticket/11474
<ul>
<li><strong>attachment</strong>
set to <em>trac11474_unique_elliptic_curves.patch</em>
</li>
</ul>
<p>
Make elliptic curves unique parents -- doesn't work yet
</p>
TicketSimonKingWed, 15 Jun 2011 13:22:58 GMTattachment set
https://trac.sagemath.org/ticket/11474
https://trac.sagemath.org/ticket/11474
<ul>
<li><strong>attachment</strong>
set to <em>test11474.log</em>
</li>
</ul>
<p>
doctest log
</p>
TicketSimonKingWed, 15 Jun 2011 13:33:13 GMTstatus changed; author set
https://trac.sagemath.org/ticket/11474#comment:4
https://trac.sagemath.org/ticket/11474#comment:4
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>needs_info</em>
</li>
<li><strong>author</strong>
set to <em>Simon King</em>
</li>
</ul>
<p>
The discussion on <a class="ext-link" href="http://groups.google.com/group/sage-nt/browse_thread/thread/ec8d0ad14a819082"><span class="icon"></span>sage-nt</a> shows that it is even not clear whether we want elliptic curves to be unique parents (yet).
</p>
<p>
Anyway, I posted a preliminary patch. There are some doctest failures -- a test log is attached as well.
</p>
<p>
Some of the errors are easy to understand: Previously, one had a "Generic morphism", but with unique parents one has a "Generic endomorphism". Others seem more tricky. For example one gets
</p>
<pre class="wiki">sage -t devel/sage/sage/schemes/elliptic_curves/ell_field.py
// ** nInitExp failed: using Z/2^2
**********************************************************************
File "/mnt/local/king/SAGE/sage-4.7.rc2/devel/sage-main/sage/schemes/elliptic_curves/ell_field.py", line 1038
:
sage: E.weierstrass_p(prec=8, algorithm='pari')
Exception raised:
Traceback (most recent call last):
File "/mnt/local/king/SAGE/sage-4.7.rc2/local/bin/ncadoctest.py", line 1231, in run_one_test
self.run_one_example(test, example, filename, compileflags)
File "/mnt/local/king/SAGE/sage-4.7.rc2/local/bin/sagedoctest.py", line 38, in run_one_example
OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags)
File "/mnt/local/king/SAGE/sage-4.7.rc2/local/bin/ncadoctest.py", line 1172, in run_one_example
compileflags, 1) in test.globs
File "<doctest __main__.example_14[7]>", line 1, in <module>
E.weierstrass_p(prec=Integer(8), algorithm='pari')###line 1038:
sage: E.weierstrass_p(prec=8, algorithm='pari')
File "/mnt/local/king/SAGE/sage-4.7.rc2/local/lib/python/site-packages/sage/schemes/elliptic_curves/ell
_field.py", line 1064, in weierstrass_p
return weierstrass_p(self, prec=prec, algorithm=algorithm)
File "/mnt/local/king/SAGE/sage-4.7.rc2/local/lib/python/site-packages/sage/schemes/elliptic_curves/ell_wp.py", line 141, in weierstrass_p
wp = compute_wp_pari(E, prec)
File "/mnt/local/king/SAGE/sage-4.7.rc2/local/lib/python/site-packages/sage/schemes/elliptic_curves/ell_wp.py", line 168, in compute_wp_pari
wpp = ep.ellwp(n=prec)
AttributeError: 'dict' object has no attribute 'ellwp'
</pre><p>
Hence, first there is a (warning or error?) message "<code>// ** nInitExp failed: using Z/2^2</code>", and then something is a dict that ought to be a completely different object.
</p>
<p>
On the bright side, one has the following.
</p>
<p>
Elliptic curves are unique, even when they are created in different ways:
</p>
<pre class="wiki">sage: E = EllipticCurve('5077a'); E
Elliptic Curve defined by y^2 + y = x^3 - 7*x + 6 over Rational Field
sage: E is EllipticCurve('5077a') is EllipticCurve(QQ, E.a_invariants()) is EllipticCurve(j = E.j_invariant())
True
</pre><p>
If an elliptic curve is provided with some custom attribute and the "same" curve is found in the database with different attributes, then one has no uniqueness of parents; this is in order to prevent the database from overriding stuff that the user has computed:
</p>
<pre class="wiki">sage: E = EllipticCurve([0, 1, 1, -2, 0])
sage: E is EllipticCurve('389a')
True
sage: E._EllipticCurve_rational_field__cremona_label = 'bogus'
sage: E is EllipticCurve('389a')
False
sage: E.label()
'bogus'
sage: EllipticCurve('389a').label()
'389a1'
</pre><p>
However, if the custom attribute is removed, then the data from the database are used to provide a value for that attribute:
</p>
<pre class="wiki">sage: del E._EllipticCurve_rational_field__cremona_label
sage: E is EllipticCurve('389a') # uniqueness of parents again
True
# The label has implicitly been updated
sage: E._EllipticCurve_rational_field__cremona_label
'389 a 1'
</pre>
TicketjdemeyerTue, 13 Aug 2013 15:35:53 GMTmilestone changed
https://trac.sagemath.org/ticket/11474#comment:5
https://trac.sagemath.org/ticket/11474#comment:5
<ul>
<li><strong>milestone</strong>
changed from <em>sage-5.11</em> to <em>sage-5.12</em>
</li>
</ul>
Ticketvbraun_spamThu, 30 Jan 2014 21:20:52 GMTmilestone changed
https://trac.sagemath.org/ticket/11474#comment:6
https://trac.sagemath.org/ticket/11474#comment:6
<ul>
<li><strong>milestone</strong>
changed from <em>sage-6.1</em> to <em>sage-6.2</em>
</li>
</ul>
TicketpbruinMon, 07 Apr 2014 10:26:56 GMTcc set
https://trac.sagemath.org/ticket/11474#comment:7
https://trac.sagemath.org/ticket/11474#comment:7
<ul>
<li><strong>cc</strong>
<em>cremona</em> added
</li>
</ul>
<p>
I think this would still be good to fix. The consensus seems to be that an <code>EllipticCurve</code> object should be defined uniquely by its base ring and the coefficients <em>a</em><sub><em>i</em></sub>, and nothing else. The datum consisting of the base ring and the Weierstrass coefficients is "enough" in the sense that if these are identical for two given elliptic curves, then there is a canonical isomorphism between them.
</p>
<p>
However, in the <code>sage-nt</code> discussion linked to above, there was some discussion about situations like the following. The user creates an elliptic curve <em>E</em> (say over <strong>Q</strong>) without using Cremona's database, and computes generators for its Mordell-Weil group. Now he later tries to load the same curve from the database, but the generators don't agree. Should the two curves be identical or not?
</p>
<p>
Intuitively, I would say they <em>should</em> be identical; the fact that there is no canonical basis for the MW group does not justify breaking the unique parents convention (equality of parents, as determined by some defining data, implies identity).
</p>
<p>
On the question of whether the generators from the database should override the ones computed by the user, I think the answer is no. The database provides a convenient way to avoid a possibly long computation, but the basis returned by it has no mathematical property that makes it preferred over any other one.
</p>
<p>
Compare to <code>NumberField</code>: it seems strange to call two number fields non-identical if they agree in all respects except that different bases for the unit group have been computed. (Actually, <code>NumberField</code> may be a bad example because the current implementation also uses various other parameters to decide whether to construct a new instance. The point is that it does <em>not</em> take any basis for the unit group into account.)
</p>
TicketpbruinMon, 07 Apr 2014 10:30:11 GMT
https://trac.sagemath.org/ticket/11474#comment:8
https://trac.sagemath.org/ticket/11474#comment:8
<p>
There is also the related question on whether an elliptic curve <em>E</em> itself or the Abelian group of rational points of <em>E</em> should be regarded as the parent of the rational points of <em>E</em>. Same question for the (co)domain of isogenies; see <a class="closed ticket" href="https://trac.sagemath.org/ticket/12880" title="defect: Inconsistent domain, codomain and parent in EllipticCurveIsogeny (closed: fixed)">#12880</a>.
</p>
TicketdefeoFri, 11 Apr 2014 22:04:30 GMTcc changed
https://trac.sagemath.org/ticket/11474#comment:9
https://trac.sagemath.org/ticket/11474#comment:9
<ul>
<li><strong>cc</strong>
<em>defeo</em> <em>sbesnier</em> added
</li>
</ul>
TicketdefeoFri, 11 Apr 2014 22:45:36 GMT
https://trac.sagemath.org/ticket/11474#comment:10
https://trac.sagemath.org/ticket/11474#comment:10
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/11474#comment:8" title="Comment 8">pbruin</a>:
</p>
<blockquote class="citation">
<p>
There is also the related question on whether an elliptic curve <em>E</em> itself or the Abelian group of rational points of <em>E</em> should be regarded as the parent of the rational points of <em>E</em>. Same question for the (co)domain of isogenies; see <a class="closed ticket" href="https://trac.sagemath.org/ticket/12880" title="defect: Inconsistent domain, codomain and parent in EllipticCurveIsogeny (closed: fixed)">#12880</a>.
</p>
</blockquote>
<p>
What happened to the idea of having the MW group as an object different from the elliptic curve? Possibly attaching different representations of it to the same curve? The sage-nt discussion mentions it, but I don't see any trace of it in the patch.
</p>
<p>
I really like the idea, as I like the general idea of having the Abelian group of (rational?) points as a separate object.
</p>
<p>
Sébastien (cc-ed) is working on this (and the related questions with isogenies) for its masters' thesis. If there is a consensus on having a separate object for the Abelian group, I think he will be happy (or at least his advisor will :) to do it and finalize this ticket.
</p>
TicketcremonaSat, 12 Apr 2014 08:20:40 GMT
https://trac.sagemath.org/ticket/11474#comment:11
https://trac.sagemath.org/ticket/11474#comment:11
<p>
I also like the idea of having a separate object for the group of points. Then for an elliptic curve E defined over K one could have different point groups for E(K) and for E(L) for extensions L of K. Magma did that a few years ago and it was very convenient. I have not thought at all about how to implement it though.
</p>
<p>
About isogeny (c)domains it is not so clear to me. An isogeny is both a map from one curve to another (preserving the base point), and also a group homomorphism. If we are going to separate out the curve E from its group(s) E(K), E(L), then should we not keep these separate for isogenies too?
</p>
TicketpbruinSat, 12 Apr 2014 09:54:46 GMT
https://trac.sagemath.org/ticket/11474#comment:12
https://trac.sagemath.org/ticket/11474#comment:12
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/11474#comment:11" title="Comment 11">cremona</a>:
</p>
<blockquote class="citation">
<p>
I also like the idea of having a separate object for the group of points. Then for an elliptic curve E defined over K one could have different point groups for E(K) and for E(L) for extensions L of K. Magma did that a few years ago and it was very convenient. I have not thought at all about how to implement it though.
</p>
</blockquote>
<p>
There is already a very basic class in <code>sage.schemes.projective_homset</code>; you can do
</p>
<pre class="wiki">sage: E=EllipticCurve('37a1')
sage: K.<a>=QuadraticField(57)
sage: E(K)
Abelian group of points on Elliptic Curve defined by y^2 + y = x^3 + (-1)*x over Number Field in a with defining polynomial x^2 - 57
sage: type(E(K))
<class 'sage.schemes.projective.projective_homset.SchemeHomset_points_abelian_variety_field_with_category'>
sage: P=(0,0)
sage: E(P) == E(QQ)(P)
True
</pre><p>
We could create a subclass of <code>SchemeHomset_points_abelian_variety</code> for groups of points of elliptic curves, and then separate subclasses of that for different base fields (rationals, general number fields, finite fields, maybe the complex field, etc.) Then we could start moving functionality so that <code>E.rank()</code> is defined as <code>E.group_of_points(E.base_field()).rank()</code>, etc.
</p>
<p>
(In the above example it looks as if <code>E(K)</code> is interpreted as the group of <code>K</code>-points of <code>E.base_extend(K)</code> rather than as the group of <code>K</code>-points of <code>E</code>, but this could probably be changed.)
</p>
<blockquote class="citation">
<p>
About isogeny (c)domains it is not so clear to me. An isogeny is both a map from one curve to another (preserving the base point), and also a group homomorphism. If we are going to separate out the curve E from its group(s) E(K), E(L), then should we not keep these separate for isogenies too?
</p>
</blockquote>
<p>
Yes, I think we should. To rephrase your point, an isogeny <em>is</em> a map f: E -> E' between elliptic curves over a field K (preserving 0), and <em>induces</em> group homomorphisms f(L): E(L) -> E'(L) for every extension L/K. (In categorical language, E determines a functor from K-algebras to Abelian groups, and f determines a morphism [natural transformation] between two such functors.)
</p>
<p>
Of course, as a Sage user you should still be able to construct a point P from its coordinates (u, v) by typing <code>P = E(u, v)</code> as an alternative for <code>P = E(K)(u, v)</code>, and likewise you should be able to apply an isogeny f to P by typing either <code>f(P)</code> or <code>f(K)(P)</code>.
</p>
TicketsbesnierMon, 14 Apr 2014 19:06:03 GMT
https://trac.sagemath.org/ticket/11474#comment:13
https://trac.sagemath.org/ticket/11474#comment:13
<p>
So, if I make it short:
</p>
<ul><li>the idea would be to generalize the "abelian_group" method which is currently only avaible for EC on Galois fields to all the EC in order to have an actual "<a class="missing wiki">AbelianGroup?</a>" in the sage category system (in addition with "Scheme"). Hence, the "non-uniqueness of set of generators" problem is moved into this "new" class of "Abelian Group of points on EC..." and it is easy to make EC unique.
</li><li>Moreover, it would be nice to do the same separation between isogenies seen as scheme-morphim and isogenis seen as group-morphim.
</li></ul><p>
Am I right? May I begin to refactoring the code in this way?
</p>
<p>
I've disscussed today with Luca today and he thinks that would be good to create a new category "<a class="missing wiki">AbelianVarieties?</a>" which require to implement this "abelian_group" method. What do you think about that ?
</p>
TicketpbruinTue, 15 Apr 2014 22:53:15 GMT
https://trac.sagemath.org/ticket/11474#comment:14
https://trac.sagemath.org/ticket/11474#comment:14
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/11474#comment:13" title="Comment 13">sbesnier</a>:
</p>
<blockquote class="citation">
<p>
So, if I make it short:
</p>
<ul><li>the idea would be to generalize the "abelian_group" method which is currently only avaible for EC on Galois fields to all the EC in order to have an actual "<a class="missing wiki">AbelianGroup?</a>" in the sage category system (in addition with "Scheme"). Hence, the "non-uniqueness of set of generators" problem is moved into this "new" class of "Abelian Group of points on EC..." and it is easy to make EC unique.
</li><li>Moreover, it would be nice to do the same separation between isogenies seen as scheme-morphim and isogenis seen as group-morphim.
</li></ul><p>
Am I right? May I begin to refactoring the code in this way?
</p>
</blockquote>
<p>
I think this sounds like a good plan. It does have a certain the risk of becoming a big project, so be careful to split it into well-defined steps and to proceed one step at a time.
</p>
<p>
Just some thoughts about the first of your two points for now. Before doing any coding, I would say it is essential to have a clear picture of both the mathematical objects and the Sage (or Python) objects representing them, and of the relations between them. Currently, given an elliptic curve <em>E</em> over a field <em>K</em>, there are at least three kinds of objects to consider:
</p>
<ul><li>the Sage object <code>E</code> representing the curve itself as a scheme (type <code>EllipticCurve_rational_field</code> or similar);
</li><li>the group of points <code>E(K)</code> (or more generally <code>E(L)</code> for an extension <code>L</code> of <code>K</code>), which does not care about the group structure but is the parent of (the Sage objects representing the) <em>K</em>-rational points of <em>E</em> (type <code>SchemeHomset_points_abelian_variety_field</code>);
</li><li>if <em>K</em> is a finite field, there is also <code>E.abelian_group()</code>, which returns an object of type <code>AdditiveAbelianGroupWrapper</code>. This represents the group structure of <code>E(K)</code> concretely by returning a product of at most two finite cyclic groups and an embedding of this product into the "abstract" group <code>E(K)</code> (which in this case is an isomorphism).
</li></ul><p>
This looks like the right framework for elliptic curves over number fields as well, since the group of <em>K</em>-points is finitely generated. For general fields (e.g. <strong>C</strong>), it only makes sense to have the first two kinds of objects. So writing a variant of <code>E.abelian_group()</code> for elliptic curves over number fields could be a logical first step.
</p>
<p>
In general it is important to find out what functionality is already available, and how much of it you can use. (Remember that a guiding principle of Sage is "building the car, not reinventing the wheel". This originally refers to the fact that Sage builds on many other pieces of software, but it is also a good general slogan.)
</p>
<blockquote class="citation">
<p>
I've disscussed today with Luca today and he thinks that would be good to create a new category "<a class="missing wiki">AbelianVarieties?</a>" which require to implement this "abelian_group" method. What do you think about that ?
</p>
</blockquote>
<p>
It would indeed be nice to have a category of Abelian varieties, but this is probably largely independent of the rest of what you want to do. The current implementation of <code>EllipticCurve_finite_field.abelian_group()</code> does not refer to categories either. I would guess it is easiest to do the other things first without involving a category of Abelian varieties (in the code; you can of course always keep it in mind as the place where things live mathematically). If you need to specify a category somewhere, <code>Schemes(K)</code> will probably be good enough for now, since this is the category in which elliptic curves over <em>K</em> currently live.
</p>
TicketdefeoTue, 15 Apr 2014 23:05:27 GMT
https://trac.sagemath.org/ticket/11474#comment:15
https://trac.sagemath.org/ticket/11474#comment:15
<blockquote class="citation">
<ul><li>the group of points <code>E(K)</code> (or more generally <code>E(L)</code> for an extension <code>L</code> of <code>K</code>), which does not care about the group structure but is the parent of (the Sage objects representing the) <em>K</em>-rational points of <em>E</em> (type <code>SchemeHomset_points_abelian_variety_field</code>);
</li><li>if <em>K</em> is a finite field, there is also <code>E.abelian_group()</code>, which returns an object of type <code>AdditiveAbelianGroupWrapper</code>.
</li></ul></blockquote>
<p>
I haven't dug enough into the way Sage represents groups, but do these two objects need to be different ? If groups in Sage need to have explicitly set generators, then obviously yes: we don't want to compute generators unless the user asks us to.
</p>
<p>
But if a group can be represented abstractly (by its elements and an algorithm for the group operation), then it seems to me that these two should be the same, with the generators computed lazily when the user asks for them.
</p>
TicketpbruinTue, 15 Apr 2014 23:31:36 GMT
https://trac.sagemath.org/ticket/11474#comment:16
https://trac.sagemath.org/ticket/11474#comment:16
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/11474#comment:15" title="Comment 15">defeo</a>:
</p>
<blockquote class="citation">
<blockquote class="citation">
<ul><li>the group of points <code>E(K)</code> (or more generally <code>E(L)</code> for an extension <code>L</code> of <code>K</code>), which does not care about the group structure but is the parent of (the Sage objects representing the) <em>K</em>-rational points of <em>E</em> (type <code>SchemeHomset_points_abelian_variety_field</code>);
</li><li>if <em>K</em> is a finite field, there is also <code>E.abelian_group()</code>, which returns an object of type <code>AdditiveAbelianGroupWrapper</code>.
</li></ul></blockquote>
<p>
I haven't dug enough into the way Sage represents groups, but do these two objects need to be different ? If groups in Sage need to have explicitly set generators, then obviously yes: we don't want to compute generators unless the user asks us to.
But if a group can be represented abstractly (by its elements and an algorithm for the group operation), then it seems to me that these two should be the same, with the generators computed lazily when the user asks for them.
</p>
</blockquote>
<p>
But don't you then run into the same difficulty with unique representation that we now have for elliptic curves, namely caching non-canonical data inside a canonical object? If you keep the two objects (i.e. the abstract group vs. the generators) separate, then it is clear that the way for the user to remember a choice of generators is to store the result of <code>E.abelian_group()</code> as opposed to the result of <code>E(K)</code>.
</p>
<p>
Also, I would say it is conceptually better to keep them separate, since the <code>AdditiveAbelianGroupWrapper</code> is actually not a group at all, but a homomorphism from some "standard" Abelian group (product of cyclic groups) to another group, in this case <code>E(K)</code>.
</p>
TicketcremonaWed, 16 Apr 2014 08:10:36 GMT
https://trac.sagemath.org/ticket/11474#comment:17
https://trac.sagemath.org/ticket/11474#comment:17
<p>
I agree with Peter here (we have been discussing these issues in our common room in Warwick).
</p>
<p>
I particularly want to emphasize that what you have in mind is quite a large project, which needs to be done step by step, with plenty of input from other experienced Sage users who will be greatly affected by such changes (e.g. me!). Also, be careful that the improved design will not be accompanied by a decrease in efficiency.
</p>
<p>
Some of what is being proposed here would have been done a lot earlier, but Sage's support for f.g. abelian groups was not good enough in the early days. As soon as it became good enough it was applied to the simplest of these cases, namely the group of points of an e.c. over a finite field.
</p>
<p>
We want to keep separate the abstract abelian group from the concrete realization of it as a set of points, so that we can use generaic abelian group functionality to (for example) compute kernels. And as well as the easy map from the abstract group to the concrete point set (using known generators) we must also provide the harder map in the reverse direction, which is a form of elliptic logarithm. This is actually easier in the number field case since we can use canonical heights to express a point as a linear combination of the generators.
</p>
TicketsbesnierWed, 16 Apr 2014 19:10:31 GMT
https://trac.sagemath.org/ticket/11474#comment:18
https://trac.sagemath.org/ticket/11474#comment:18
<p>
Considering the EC part, we already have almost what we want isn't it?
</p>
<p>
Let <code>E=EllipticCurve(whatever)</code>, we "only" have to:
</p>
<ul><li>make <code>E</code> unique : it is already almost done by Simon's patch, although this patch maintains the gens in <code>E</code>. Meanwhile, the files had been edited for other reasons; will nevertheless the automatic tools work or might I manually edit the files thanks to the diff in order to reuse Simon's work?
</li><li>turn <code>E(K)</code> into an actual group
</li><li>extend the support of <code>E.abelian_group(K)</code> when K is a number field (and move the method in E(K)?)
</li><li>move <code>.rank()</code>, <code>.gens()</code> and other non-canonical methods/attribute to `E.abelian_group(K)
</li></ul><p>
John wrote:
</p>
<blockquote class="citation">
<p>
And as well as the easy map from the abstract group to the concrete point set (using known generators) we must also provide the harder map in the reverse direction, which is a form of elliptic logarithm.
</p>
</blockquote>
<p>
I'm not sure to see the point. You would like to have the map <code>E(K) -> E.abelian_group()</code> which calculate calculate the generators, and also the map <code>E.abelian_group() -> E (or E(K)?)</code> which calculates the a-invariants of E (or anything which determines E uniquely) from the generators?
</p>
TicketcremonaWed, 16 Apr 2014 19:44:00 GMT
https://trac.sagemath.org/ticket/11474#comment:19
https://trac.sagemath.org/ticket/11474#comment:19
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/11474#comment:18" title="Comment 18">sbesnier</a>:
</p>
<blockquote class="citation">
<p>
Considering the EC part, we already have almost what we want isn't it?
</p>
<p>
Let <code>E=EllipticCurve(whatever)</code>, we "only" have to:
</p>
<ul><li>make <code>E</code> unique : it is already almost done by Simon's patch, although this patch maintains the gens in <code>E</code>. Meanwhile, the files had been edited for other reasons; will nevertheless the automatic tools work or might I manually edit the files thanks to the diff in order to reuse Simon's work?
</li></ul></blockquote>
<p>
Don't use the patch as it is: for a start Sage no longer uses patches but git branches, and also as the patch is 3 years old it certainly will not apply cleanly. What will be needed is a new git branch based on the current development branch (also known as version 6.2.beta8) onto which the same changes as made by the patch are applied. You can try to use Sage's methods for converting patches to git branches, but it would be a miracle if that worked on a 3-year-old patch! If you are nervous, someone else might do this step for you. Then after committing Simon's changes in the new branch you can add your changes, make a new commit, and everyone will be able to try it out.
</p>
<blockquote class="citation">
<ul><li>turn <code>E(K)</code> into an actual group
</li><li>extend the support of <code>E.abelian_group(K)</code> when K is a number field (and move the method in E(K)?)
</li><li>move <code>.rank()</code>, <code>.gens()</code> and other non-canonical methods/attribute to `E.abelian_group(K)
</li></ul><p>
John wrote:
</p>
<blockquote class="citation">
<p>
And as well as the easy map from the abstract group to the concrete point set (using known generators) we must also provide the harder map in the reverse direction, which is a form of elliptic logarithm.
</p>
</blockquote>
<p>
I'm not sure to see the point. You would like to have the map <code>E(K) -> E.abelian_group()</code> which calculate calculate the generators, and also the map <code>E.abelian_group() -> E (or E(K)?)</code> which calculates the a-invariants of E (or anything which determines E uniquely) from the generators?
</p>
</blockquote>
<p>
My point is that somewhere there has to be code which, given a point and known generators, expresses that point as a Z-linear combination of the generators. Assuming that E.abelian_group was an abstract abelian group, this code would be used in mapping from E(K) to it.
</p>
<blockquote class="citation">
</blockquote>
TicketpbruinWed, 16 Apr 2014 22:09:47 GMT
https://trac.sagemath.org/ticket/11474#comment:20
https://trac.sagemath.org/ticket/11474#comment:20
<p>
One remark about the approach to follow to make elliptic curves satisfy unique representation: it turns out that there is an important problem with using <code>UniqueRepresentation</code> (which Simon's patch introduces) together with caching (which is certainly used by <code>EllipticCurve</code>), see <a class="ext-link" href="https://groups.google.com/forum/#!topic/sage-devel/q5uy_lI11jg"><span class="icon"></span>https://groups.google.com/forum/#!topic/sage-devel/q5uy_lI11jg</a>. If I remember correctly, for non-trivial parents it is recommended to use a <code>UniqueFactory</code>, which gives more flexibility than <code>UniqueRepresentation</code>. (I personally also understand <code>UniqueFactory</code> better than the <code>__classcall__()</code> magic of Simon's patch, but that may be simply because I am too familiar with this part of the internals of Sage.) Maybe Simon or someone else can elaborate on this point?
</p>
TicketjpfloriFri, 18 Apr 2014 08:52:41 GMTcc changed
https://trac.sagemath.org/ticket/11474#comment:21
https://trac.sagemath.org/ticket/11474#comment:21
<ul>
<li><strong>cc</strong>
<em>jpflori</em> added
</li>
</ul>
TicketdefeoFri, 18 Apr 2014 13:41:37 GMT
https://trac.sagemath.org/ticket/11474#comment:22
https://trac.sagemath.org/ticket/11474#comment:22
<p>
I'm trying to gitify Simon's code, but I get heaps of errors when I doctest. They all seem to be the same:
</p>
<pre class="wiki"> File "/home/dfl/sage/local/lib/python2.7/site-packages/sage/databases/cremona.py", line 873, in elliptic_curve
F = EllipticCurve_rational_field.__new__(EllipticCurve_rational_field)
TypeError: sage.misc.fast_methods.WithEqualityById.__new__(EllipticCurve_rational_field) is not safe, use sage.structure.parent.Parent.__new__()
</pre><p>
Does anyone understand it?
</p>
TicketcremonaFri, 18 Apr 2014 14:16:41 GMT
https://trac.sagemath.org/ticket/11474#comment:23
https://trac.sagemath.org/ticket/11474#comment:23
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/11474#comment:22" title="Comment 22">defeo</a>:
</p>
<blockquote class="citation">
<p>
I'm trying to gitify Simon's code, but I get heaps of errors when I doctest. They all seem to be the same:
</p>
<pre class="wiki"> File "/home/dfl/sage/local/lib/python2.7/site-packages/sage/databases/cremona.py", line 873, in elliptic_curve
F = EllipticCurve_rational_field.__new__(EllipticCurve_rational_field)
TypeError: sage.misc.fast_methods.WithEqualityById.__new__(EllipticCurve_rational_field) is not safe, use sage.structure.parent.Parent.__new__()
</pre><p>
Does anyone understand it?
</p>
</blockquote>
<p>
Not really, but what it is doing at that point of sage/databases/cremona.py is constructing an elliptic curve from some data such as the label, which causes it to look that up in the database. What is the state of your local/share/cremona/cremona_mini.db file? Is it 8537088 bytes? If not, you could try sage -f elliptic_curves.
</p>
TicketdefeoFri, 18 Apr 2014 14:27:23 GMT
https://trac.sagemath.org/ticket/11474#comment:24
https://trac.sagemath.org/ticket/11474#comment:24
<blockquote class="citation">
<p>
Not really, but what it is doing at that point of sage/databases/cremona.py is constructing an elliptic curve from some data such as the label, which causes it to look that up in the database. What is the state of your local/share/cremona/cremona_mini.db file? Is it 8537088 bytes? If not, you could try sage -f elliptic_curves.
</p>
</blockquote>
<p>
My cremona_mini.db is up to date. The error doesn't seem related to the database. In a sage shell
</p>
<pre class="wiki">sage: from sage.schemes.elliptic_curves.ell_rational_field import EllipticCurve_rational_field
sage: EllipticCurve_rational_field.__new__(EllipticCurve_rational_field)
---------------------------------------------------------------------------
...
AttributeError: 'EllipticCurve_rational_field' object has no attribute '_EllipticCurve_generic__ainvs'
</pre><p>
With Simon's patch applied (using the <code>__classcall__</code> magic all over the place).
</p>
<pre class="wiki">sage: from sage.schemes.elliptic_curves.ell_rational_field import EllipticCurve_rational_field
sage: EllipticCurve_rational_field.__new__(EllipticCurve_rational_field)
---------------------------------------------------------------------------
...
TypeError: sage.misc.fast_methods.WithEqualityById.__new__(EllipticCurve_rational_field) is not safe, use sage.structure.parent.Parent.__new__()
sage: Parent.__new__(EllipticCurve_rational_field)
---------------------------------------------------------------------------
...
AttributeError: 'EllipticCurve_rational_field' object has no attribute sage: '_EllipticCurve_generic__ainvs'
</pre><p>
Either way, I don't understand how this line is supposed to work. I think we need help from Simon, if we want to resurrect his patch.
</p>
TicketsbesnierFri, 18 Apr 2014 14:31:02 GMT
https://trac.sagemath.org/ticket/11474#comment:25
https://trac.sagemath.org/ticket/11474#comment:25
<p>
pbruin wrote:
</p>
<blockquote class="citation">
<p>
One remark about the approach to follow to make elliptic curves satisfy unique representation: it turns out that there is an important problem with using <a class="missing wiki">UniqueRepresentation?</a> (which Simon's patch introduces) together with caching (which is certainly used by <a class="missing wiki">EllipticCurve?</a>), see <a class="ext-link" href="https://groups.google.com/forum/#!topic/sage-devel/q5uy_lI11jg"><span class="icon"></span>https://groups.google.com/forum/#!topic/sage-devel/q5uy_lI11jg</a>. If I remember correctly, for non-trivial parents it is recommended to use a <a class="missing wiki">UniqueFactory?</a>, which gives more flexibility than <a class="missing wiki">UniqueRepresentation?</a>. (I personally also understand <a class="missing wiki">UniqueFactory?</a> better than the <span class="underline">classcall</span>() magic of Simon's patch, but that may be simply because I am too familiar with this part of the internals of Sage.) Maybe Simon or someone else can elaborate on this point?
</p>
</blockquote>
<p>
If I correctly understand Nils position, it would be better to use a UniqueFactory isn't it and not the UniqueRepresentation ? In that case, is it usefull to gitify this UniqueRepresentation patch ?
</p>
TicketdefeoFri, 18 Apr 2014 15:02:13 GMTchangetime changed; branch set
https://trac.sagemath.org/ticket/11474#comment:26
https://trac.sagemath.org/ticket/11474#comment:26
<ul>
<li><strong>changetime</strong>
changed from <em>04/18/14 14:31:02</em> to <em>04/18/14 14:31:02</em>
</li>
<li><strong>branch</strong>
set to <em>u/defeo/ticket/11474</em>
</li>
</ul>
TicketdefeoFri, 18 Apr 2014 15:08:08 GMTcommit set
https://trac.sagemath.org/ticket/11474#comment:27
https://trac.sagemath.org/ticket/11474#comment:27
<ul>
<li><strong>commit</strong>
set to <em>8cbb73e266096e02efc917545aab36d067b7e063</em>
</li>
</ul>
<blockquote class="citation">
<p>
If I correctly understand Nils position, it would be better to use a UniqueFactory isn't it and not the UniqueRepresentation ? In that case, is it usefull to gitify this UniqueRepresentation patch ?
</p>
</blockquote>
<p>
Probably not. Anyway, I gitified it, thus I'm pushing it, even if it doesn't work. At least, it will be easier to read Simon's proposed changes.
</p>
<p>
There were conflicts with <a class="closed ticket" href="https://trac.sagemath.org/ticket/10999" title="defect: Elliptic curve generators from the database lie on the wrong curve (closed: fixed)">#10999</a> (which is another bug related to uniqueness) in <code>ell_rational_field.py</code>. I hopefully merged them correctly.
</p>
<hr />
<p>
New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=8cbb73e266096e02efc917545aab36d067b7e063"><span class="icon"></span>8cbb73e</a></td><td><code>11474: Make elliptic curves unique parents.</code>
</td></tr></table>
TicketjpfloriFri, 18 Apr 2014 15:30:50 GMT
https://trac.sagemath.org/ticket/11474#comment:28
https://trac.sagemath.org/ticket/11474#comment:28
<p>
For what it's worth, there was a similar <span class="underline">new</span> issue a long time ago:
</p>
<ul><li><a class="ext-link" href="http://osdir.com/ml/mathematics.sage.devel/2006-09/msg00229.html"><span class="icon"></span>http://osdir.com/ml/mathematics.sage.devel/2006-09/msg00229.html</a>
</li></ul>
TicketSimonKingFri, 18 Apr 2014 15:32:28 GMT
https://trac.sagemath.org/ticket/11474#comment:29
https://trac.sagemath.org/ticket/11474#comment:29
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/11474#comment:20" title="Comment 20">pbruin</a>:
</p>
<blockquote class="citation">
<p>
If I remember correctly, for non-trivial parents it is recommended to use a <code>UniqueFactory</code>, which gives more flexibility than <code>UniqueRepresentation</code>. (I personally also understand <code>UniqueFactory</code> better than the <code>__classcall__()</code> magic of Simon's patch, but that may be simply because I am too familiar with this part of the internals of Sage.) Maybe Simon or someone else can elaborate on this point?
</p>
</blockquote>
<p>
I don't recall what I did in this patch, and I do not have the bandwith to look at it right now. So, for now I can only try to explain my general point of view to factory versus <code>CachedRepresentation</code> versus <code>UniqueRepresentation</code> versus <code>WithEqualityById</code>.
</p>
<p>
If one uses a factory, then one can rather cleanly separate the creation of the cache key from the input data, additional data used to create the object (without being relevant to the cache) and the creation of the object to be returned. What it returns is <strong>not</strong> a unique parent, because comparison and hash is (and can not) be taken care of by the factory. Drawbacks:
</p>
<ul><li>One needs to write the factory code. At least in trivial cases, <code>CachedRepresentation</code> involves less manual work.
</li><li>Suppose you used to have a factory for some parents, but later decide to remove the factory. If you remove the factory from Sage, then old pickles will break, since they rely on the factory being available. This is in contrast to <code>CachedRepresentation</code>, where you just need that the class name is preserved: If you remove <code>CachedRepresentation</code> from the base classes, then old pickles can still be opened.
</li><li>I think it is not nice that one has to do <code>MyFancyStuff(args)</code> to create an object, but then the resulting object is not an instance of <code>MyFancyStuff</code>. But that might be a matter of taste.
</li></ul><p>
<code>CachedRepresentation</code> is essentially equivalent to using a factory. In particular, it does <strong>not</strong> result in unique parents. Drawbacks:
</p>
<ul><li>It can not be used on extension classes, since a metaclass is involved (which is not supported by Cython).
</li><li>It does not cleanly separate creation of the cache, additional arguments and object creation. You can write <code>__classcall(_private)__</code> so that it does exactly the same as a factory, it can look up stuff in a database, and so on. But the code is less clean and thus more error prone.
</li></ul><p>
So, at this point, we have some kind of factory, but no uniqueness of parents. In order to add unique parent behaviour, one can simply add <code>WithEqualityById</code> to the list of base classes (and make sure that hash and comparison will not be overridden in the sub-class).
</p>
<p>
<code>UniqueRepresentation</code> simply is a ready-made combination of <code>CachedRepresentation</code> and <code>WithEqualityById</code>, there is nothing more to it.
</p>
<p>
I hope that these remarks help to assess whether it would be better to use a factory PLUS <code>WithEqualityById</code> on elliptic curves, or "simply" use <code>UniqueRepresentation</code>.
</p>
Ticketvbraun_spamTue, 06 May 2014 15:20:58 GMTmilestone changed
https://trac.sagemath.org/ticket/11474#comment:30
https://trac.sagemath.org/ticket/11474#comment:30
<ul>
<li><strong>milestone</strong>
changed from <em>sage-6.2</em> to <em>sage-6.3</em>
</li>
</ul>
TicketcremonaSat, 31 May 2014 14:11:51 GMT
https://trac.sagemath.org/ticket/11474#comment:31
https://trac.sagemath.org/ticket/11474#comment:31
<p>
There seem to be a lot of other tickets waiting for this one. It is tagged "needs info": what info, and from whom? What is the status of the branch & commits listed?
</p>
TicketSimonKingSat, 31 May 2014 14:26:55 GMT
https://trac.sagemath.org/ticket/11474#comment:32
https://trac.sagemath.org/ticket/11474#comment:32
<p>
I have put "needs_info" at some point, because it has not been clear whether we actually want unique parent behaviour for elliptic curves. Peter Bruin has replied something on that question, but I think we never came to a real conclusion.
</p>
TicketcremonaSat, 31 May 2014 16:26:30 GMT
https://trac.sagemath.org/ticket/11474#comment:33
https://trac.sagemath.org/ticket/11474#comment:33
<p>
Thanks. We need to reach a conclusion while there are other "needs review" tickets claiming to depend on this. I used to know what that meant, but now... it may or may not mean that the branches asking for review include the unreviewed commits here. For example <a class="closed ticket" href="https://trac.sagemath.org/ticket/12880" title="defect: Inconsistent domain, codomain and parent in EllipticCurveIsogeny (closed: fixed)">#12880</a>.
</p>
TicketpbruinSat, 31 May 2014 17:46:03 GMT
https://trac.sagemath.org/ticket/11474#comment:34
https://trac.sagemath.org/ticket/11474#comment:34
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/11474#comment:33" title="Comment 33">cremona</a>:
</p>
<blockquote class="citation">
<p>
Thanks. We need to reach a conclusion while there are other "needs review" tickets claiming to depend on this. I used to know what that meant, but now... it may or may not mean that the branches asking for review include the unreviewed commits here. For example <a class="closed ticket" href="https://trac.sagemath.org/ticket/12880" title="defect: Inconsistent domain, codomain and parent in EllipticCurveIsogeny (closed: fixed)">#12880</a>.
</p>
</blockquote>
<p>
I think the other tickets depend on this one in the sense that they need elliptic curves to be unique parents, not in the sense that they already include commits that belong to this ticket.
</p>
<p>
In my opinion we should definitely make elliptic curves unique parents. The question is whether we should use <code>UniqueRepresentation</code> or <code>UniqueFactory</code> (or a custom cache, but I don't see why that would be needed). I would personally prefer a <code>UniqueFactory</code>, mostly because I think it is more natural and uses less black magic than <code>UniqueRepresentation</code> + <code>__classcall__</code>, and hence will be easier to understand/extend for other developers.
</p>
<p>
The idea is that the <code>UniqueFactory</code> is the place where we do all the work related to converting various input data into the 5-tuple of <em>a</em>-coefficients. There are <em>many</em> possible input data, for example:
</p>
<ul><li><em>a</em>-coefficients
</li><li>base ring + <em>a</em>-coefficients
</li><li><em>c</em>-coefficients
</li><li>Cremona label
</li><li>base ring + Cremona label
</li><li><em>j</em>-invariant
</li></ul><p>
The logic of converting any of these input data into <em>a</em>-coefficients is very similar for the various base rings, so it seems best to do it all in one place. I imagine that the future <code>UniqueFactory</code> would do the work of the current (user-level) function <code>sage.schemes.elliptic_curves.EllipticCurve</code> plus the handling of Cremona labels, which is currently duplicated [triplicated?] in <code>EllipticCurve_rational_field</code>, <code>EllipticCurve_number_field</code> and <code>EllipticCurve_padic_field</code>.
</p>
<p>
Of course, whoever implements this in the end has to make the choice how to do it. Unfortunately I am currently a bit too busy with some research things and don't have a lot of time for it at the moment.
</p>
TicketSimonKingSat, 31 May 2014 17:54:24 GMT
https://trac.sagemath.org/ticket/11474#comment:35
https://trac.sagemath.org/ticket/11474#comment:35
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/11474#comment:34" title="Comment 34">pbruin</a>:
</p>
<blockquote class="citation">
<p>
In my opinion we should definitely make elliptic curves unique parents. The question is whether we should use <code>UniqueRepresentation</code> or <code>UniqueFactory</code> (or a custom cache, but I don't see why that would be needed). I would personally prefer a <code>UniqueFactory</code>, mostly because I think it is more natural and uses less black magic than <code>UniqueRepresentation</code> + <code>__classcall__</code>, and hence will be easier to understand/extend for other developers.
</p>
</blockquote>
<p>
... except for the fact that <code>UniqueFactory</code> <em>alone</em> is not enough to create a unique parent. You need to additionally adapt <code>__cmp__</code> etc, and this is what inheritance from <code>UniqueRepresentation</code> does on top of <code>__classcall__</code>.
</p>
<blockquote class="citation">
<p>
The idea is that the <code>UniqueFactory</code> is the place where we do all the work related to converting various input data into the 5-tuple of <em>a</em>-coefficients. There are <em>many</em> possible input data, for example:
</p>
<ul><li><em>a</em>-coefficients
</li><li>base ring + <em>a</em>-coefficients
</li><li><em>c</em>-coefficients
</li><li>Cremona label
</li><li>base ring + Cremona label
</li><li><em>j</em>-invariant
</li></ul></blockquote>
<p>
This I consider a valid argument for using a factory. But a tad more needs to be done.
</p>
TicketpbruinSat, 31 May 2014 18:27:44 GMT
https://trac.sagemath.org/ticket/11474#comment:36
https://trac.sagemath.org/ticket/11474#comment:36
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/11474#comment:35" title="Comment 35">SimonKing</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/11474#comment:34" title="Comment 34">pbruin</a>:
</p>
<blockquote class="citation">
<p>
I would personally prefer a <code>UniqueFactory</code>, mostly because I think it is more natural and uses less black magic than <code>UniqueRepresentation</code> + <code>__classcall__</code>, and hence will be easier to understand/extend for other developers.
</p>
</blockquote>
<p>
... except for the fact that <code>UniqueFactory</code> <em>alone</em> is not enough to create a unique parent. You need to additionally adapt <code>__cmp__</code> etc, and this is what inheritance from <code>UniqueRepresentation</code> does on top of <code>__classcall__</code>.
</p>
</blockquote>
<p>
Something that I think you recently clarified to me elsewhere, and which might be worth emphasising here too, is that "unique parents" really refers to <em>two</em> conditions:
</p>
<ul><li>equivalent input data (in the sense that we regard the resulting objects as being "the same", e.g. <em>a</em>-coefficients vs. Cremona label corresponding to the same curve) should result in identical parents;
</li><li>parents should compare equal if and only if they are identical.
</li></ul><p>
I was thinking about the first point, which is the more difficult one here. If we end up solving this by implementing a <code>UniqueFactory</code>, then the second point can be solved by simply inheriting from <code>WithEqualityById</code>, right?
</p>
TicketSimonKingSat, 31 May 2014 19:08:46 GMT
https://trac.sagemath.org/ticket/11474#comment:37
https://trac.sagemath.org/ticket/11474#comment:37
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/11474#comment:36" title="Comment 36">pbruin</a>:
</p>
<blockquote class="citation">
<ul><li>equivalent input data (in the sense that we regard the resulting objects as being "the same", e.g. <em>a</em>-coefficients vs. Cremona label corresponding to the same curve) should result in identical parents;
</li><li>parents should compare equal if and only if they are identical.
</li></ul><p>
I was thinking about the first point, which is the more difficult one here. If we end up solving this by implementing a <code>UniqueFactory</code>, then the second point can be solved by simply inheriting from <code>WithEqualityById</code>, right?
</p>
</blockquote>
<p>
Yes.
</p>
TicketcremonaSat, 31 May 2014 19:21:55 GMT
https://trac.sagemath.org/ticket/11474#comment:38
https://trac.sagemath.org/ticket/11474#comment:38
<p>
Sounds like quite a big job, needing people who know enough about elliptic curves and also enough about <a class="missing wiki">UniqueFactories?</a>. Perhaps we can recruit Nils Bruin to help?
</p>
TicketpbruinMon, 23 Jun 2014 11:30:44 GMTdependencies set
https://trac.sagemath.org/ticket/11474#comment:39
https://trac.sagemath.org/ticket/11474#comment:39
<ul>
<li><strong>dependencies</strong>
set to <em>#10665, #16317</em>
</li>
</ul>
<p>
I have been working on a branch that makes elliptic curves unique using <code>UniqueFactory</code>. It simultaneously cleans up the elliptic curve construction code and the relation of Sage elliptic curves to the data in Cremona's database.
</p>
<p>
My approach removes the need for a separate "database curve" attached to an elliptic curve over <strong>Q</strong>. With my branch, an elliptic curve constructed from the database is <em>identical</em> to the same curve constructed using Weierstrass coefficients. Moreover, the curves returned by <code>E.database_curve()</code> and <code>E.minimal_model()</code> are now identical, and both of them are identical to <em>E</em> if <em>E</em> is already in canonical minimal form.
</p>
<p>
I have to add some documentation and will upload my branch soon. There are unfortunately two remaining doctest failures, which are manifestations of <a class="closed ticket" href="https://trac.sagemath.org/ticket/10665" title="defect: bug in elliptic curve two_descent command (closed: fixed)">#10665</a> and <a class="closed ticket" href="https://trac.sagemath.org/ticket/16317" title="enhancement: UniqueFactory for unhashable elements (closed: fixed)">#16317</a>.
</p>
TicketpbruinWed, 25 Jun 2014 09:38:59 GMTstatus, commit, branch, author changed
https://trac.sagemath.org/ticket/11474#comment:40
https://trac.sagemath.org/ticket/11474#comment:40
<ul>
<li><strong>status</strong>
changed from <em>needs_info</em> to <em>needs_review</em>
</li>
<li><strong>commit</strong>
changed from <em>8cbb73e266096e02efc917545aab36d067b7e063</em> to <em>9871e7fa0e194b6e21a2f7a5f2d727bbd855ff88</em>
</li>
<li><strong>branch</strong>
changed from <em>u/defeo/ticket/11474</em> to <em>u/pbruin/11474-elliptic_curves_unique</em>
</li>
<li><strong>author</strong>
changed from <em>Simon King</em> to <em>Peter Bruin</em>
</li>
</ul>
TicketgitWed, 16 Jul 2014 19:48:17 GMTcommit changed
https://trac.sagemath.org/ticket/11474#comment:41
https://trac.sagemath.org/ticket/11474#comment:41
<ul>
<li><strong>commit</strong>
changed from <em>9871e7fa0e194b6e21a2f7a5f2d727bbd855ff88</em> to <em>f7f37555460bcf62c657cba260f7151a9d5c6576</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=f7f37555460bcf62c657cba260f7151a9d5c6576"><span class="icon"></span>f7f3755</a></td><td><code>Trac 11474: fix doctest that depended on the extended Cremona database</code>
</td></tr></table>
TicketvbraunThu, 17 Jul 2014 00:08:30 GMTreviewer set
https://trac.sagemath.org/ticket/11474#comment:42
https://trac.sagemath.org/ticket/11474#comment:42
<ul>
<li><strong>reviewer</strong>
set to <em>Volker Braun</em>
</li>
</ul>
TicketvbraunThu, 17 Jul 2014 13:57:32 GMTstatus, branch changed; resolution set
https://trac.sagemath.org/ticket/11474#comment:43
https://trac.sagemath.org/ticket/11474#comment:43
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>closed</em>
</li>
<li><strong>resolution</strong>
set to <em>fixed</em>
</li>
<li><strong>branch</strong>
changed from <em>u/pbruin/11474-elliptic_curves_unique</em> to <em>f7f37555460bcf62c657cba260f7151a9d5c6576</em>
</li>
</ul>
TicketjdemeyerFri, 28 Nov 2014 14:43:56 GMTcommit deleted
https://trac.sagemath.org/ticket/11474#comment:44
https://trac.sagemath.org/ticket/11474#comment:44
<ul>
<li><strong>commit</strong>
<em>f7f37555460bcf62c657cba260f7151a9d5c6576</em> deleted
</li>
</ul>
<p>
Follow-up: <a class="closed ticket" href="https://trac.sagemath.org/ticket/17415" title="defect: Random failure in ell_rational_field.py (closed: fixed)">#17415</a>.
</p>
Ticket