Sage: Ticket #16743: Extend IsogenyClass_EC to work over number fields
https://trac.sagemath.org/ticket/16743
<p>
If E is an elliptic curve over QQ then E.isogeny_class() is an object of type (class) IsogenyClass_EC which which one can work in a nice way. This should be extended to number fields. Two reasons why this is notn-trivial (and hence was not done at the same time as over QQ): (1) bounding the possible primes degrees of isogenies; (2) handling curves with CM. For (1), the module <code>gal_reps_number_field</code> does what is required. For (2) new code has been written.
</p>
en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/16743
Trac 1.1.6vbraun_spamSun, 10 Aug 2014 16:51:03 GMTmilestone changed
https://trac.sagemath.org/ticket/16743#comment:1
https://trac.sagemath.org/ticket/16743#comment:1
<ul>
<li><strong>milestone</strong>
changed from <em>sage-6.3</em> to <em>sage-6.4</em>
</li>
</ul>
TicketcremonaMon, 11 Aug 2014 11:19:44 GMTdependencies set
https://trac.sagemath.org/ticket/16743#comment:2
https://trac.sagemath.org/ticket/16743#comment:2
<ul>
<li><strong>dependencies</strong>
set to <em>#11327, #16764</em>
</li>
</ul>
TicketcremonaWed, 13 Aug 2014 14:26:39 GMTstatus changed; commit, branch set
https://trac.sagemath.org/ticket/16743#comment:3
https://trac.sagemath.org/ticket/16743#comment:3
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>needs_review</em>
</li>
<li><strong>commit</strong>
set to <em>3c96a7131c23e700d7311162361c3c77fa5bef95</em>
</li>
<li><strong>branch</strong>
set to <em>u/cremona/ticket/16743</em>
</li>
</ul>
<p>
Last 10 new commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=8dff2965d46e6f1e6b25d7b9b950f3d3525a7418"><span class="icon"></span>8dff296</a></td><td><code>#16764: CM detection over number fields</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=caf41e68e76a753268ce4f4dae36555f0003017b"><span class="icon"></span>caf41e6</a></td><td><code>#16764 revert changes to isogeny_class.py which belong in #16743</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=93ef887148635ea9e9c7379c3be8b7135bb53f4c"><span class="icon"></span>93ef887</a></td><td><code>Merge branch 'develop' into isogs-ecnf</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=e043ae8eeb0ed3b12f748131904d5e0e0476a83e"><span class="icon"></span>e043ae8</a></td><td><code>#11327,#16779: improvements to isogeny class internals and docstrings</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=7763359bf664835e0b2fad18d7b7e033a8f6e905"><span class="icon"></span>7763359</a></td><td><code>Merge branch 'isogs' into isogs-ecnf</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=c89a6a620cdd484bce61f81cfbc7429c53438e60"><span class="icon"></span>c89a6a6</a></td><td><code>#16743 in progress</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=8e203af8213626f39376cfca7742943bbb19c8c4"><span class="icon"></span>8e203af</a></td><td><code>Merge branch 'develop' into cm</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=3cacdb2fe8a32114e13c26f50c3556800b4668fe"><span class="icon"></span>3cacdb2</a></td><td><code>#16764: adjustments after review</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=8a7495952d74fd046c36d4cfa87d70341dd969b5"><span class="icon"></span>8a74959</a></td><td><code>Merge remote-tracking branch 'trac/u/cremona/ticket/16764' into isogs-ecnf</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=3c96a7131c23e700d7311162361c3c77fa5bef95"><span class="icon"></span>3c96a71</a></td><td><code>#16743: elliptic curve isogeny classes over number fields</code>
</td></tr></table>
TicketcremonaWed, 13 Aug 2014 14:31:12 GMT
https://trac.sagemath.org/ticket/16743#comment:4
https://trac.sagemath.org/ticket/16743#comment:4
<p>
To potential reviewers: don't be put off by the large number of commits listed, that is just because of the dependencies. Once the dependencies are accepted there is just one commit which is relevant (3c96a71 <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>: elliptic curve isogeny classes over number fields).
</p>
TicketgitWed, 27 Aug 2014 14:07:17 GMTcommit changed
https://trac.sagemath.org/ticket/16743#comment:5
https://trac.sagemath.org/ticket/16743#comment:5
<ul>
<li><strong>commit</strong>
changed from <em>3c96a7131c23e700d7311162361c3c77fa5bef95</em> to <em>e54356510afe82102e9dc66a08683476f0ab7d7b</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=ea410d47cdf63f26bf4b7dacf255b90b3f9d13d5"><span class="icon"></span>ea410d4</a></td><td><code>Revert "Updated Sage version to 6.3"</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=4b1a2d4ece0914fff47dae5ee4210dcf5745c217"><span class="icon"></span>4b1a2d4</a></td><td><code>Revert "Revert "Updated Sage version to 6.3""</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=12c6f0162ff02ab4800f969f23f40942c6852c0d"><span class="icon"></span>12c6f01</a></td><td><code>comitting because git wants me to?</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=dc0185c5b11d7ce7347bd4a724efc1bf7bf6a4f1"><span class="icon"></span>dc0185c</a></td><td><code>sorted out now?</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=87718324fe607636d420d0fa25874d7e1de67009"><span class="icon"></span>8771832</a></td><td><code>forgot to import Set</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=d7c33261a5b07e29a12f6fbf364d79290a199c3e"><span class="icon"></span>d7c3326</a></td><td><code>Merge remote-tracking branch 'trac/u/elarson3/isogeny_bounds_for_elliptic_curves_over_number_fields' into isogs-ecnf</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=cadd58f3290e1c9e44b86d30195ce921c0f5db95"><span class="icon"></span>cadd58f</a></td><td><code>Merge remote-tracking branch 'trac/u/elarson3/isogeny_bounds_for_elliptic_curves_over_number_fields' into larson</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=828ed6fee261a575c27e996d84c11c2bc7dabac0"><span class="icon"></span>828ed6f</a></td><td><code>#16806: reviewer's patch</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=77ac99415f04b6aa157988334f3adced4f0bd4ed"><span class="icon"></span>77ac994</a></td><td><code>Merge branch 'larson' into isogs-ecnf</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=e54356510afe82102e9dc66a08683476f0ab7d7b"><span class="icon"></span>e543565</a></td><td><code>#16743: now use reducible_primes from #16806</code>
</td></tr></table>
TicketcremonaWed, 27 Aug 2014 14:13:01 GMTdependencies changed
https://trac.sagemath.org/ticket/16743#comment:6
https://trac.sagemath.org/ticket/16743#comment:6
<ul>
<li><strong>dependencies</strong>
changed from <em>#11327, #16764</em> to <em>#11327, #16764, #16806</em>
</li>
</ul>
<p>
I added <a class="closed ticket" href="https://trac.sagemath.org/ticket/16806" title="enhancement: Isogeny Bounds for Elliptic Curves over Number Fields (closed: fixed)">#16806</a> as a dependency: that ticket concerns Gal Reps over number fields and provides an isogeny_bound() function (method) for those, to which I have added a function reducible_primes() for compatibility with the class for Gal Reps over QQ, which takes the primes in isogeny_bounds() and actually checks if there are isogenies of those degrees. At the same time I simplified the reducible_primes() method for the Gap Reps class over Q so that it does not compute the whole isogeny class but is slightly more clever, since apart from 2,3,5,7,13 one can tell by looking at a list of special j-invariants.
</p>
<p>
TODO (but not on this ticket): unify the two <a class="missing wiki">GaloisRepresentation?</a> classes! Perhaps there should be a base class which is rather abstract but defines the interface, with child classes for Galois Reps attached to (1) elliptic curves over number fields, (1') same over QQ, (2) modular forms, etc etc.
</p>
TicketgitWed, 24 Sep 2014 12:35:39 GMTcommit changed
https://trac.sagemath.org/ticket/16743#comment:7
https://trac.sagemath.org/ticket/16743#comment:7
<ul>
<li><strong>commit</strong>
changed from <em>e54356510afe82102e9dc66a08683476f0ab7d7b</em> to <em>06d9eb2226151a35bb2d3779e5309f4d066af8ca</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=3fe066e775c63857fce89c8ae7b42bed921bbaf4"><span class="icon"></span>3fe066e</a></td><td><code>Merge remote-tracking branch 'trac/u/jdemeyer/ticket/15767' into isogs-ecnf</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=0f4e30b76c98a83fdf8a060f80502a884dcc3d00"><span class="icon"></span>0f4e30b</a></td><td><code>Merge branch 'develop' into larson</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=be81f80ca53d936ef7fe763753d3e6a34c0bc944"><span class="icon"></span>be81f80</a></td><td><code>#16806: minor changes after review</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=7c93be1145e2a898b59b33b536faa47b74f9b0f9"><span class="icon"></span>7c93be1</a></td><td><code>Merge branch 'larson' into isogs-ecnf</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=06d9eb2226151a35bb2d3779e5309f4d066af8ca"><span class="icon"></span>06d9eb2</a></td><td><code>#16743: tweak to ideal comparison function</code>
</td></tr></table>
TicketcremonaWed, 24 Sep 2014 12:41:13 GMT
https://trac.sagemath.org/ticket/16743#comment:8
https://trac.sagemath.org/ticket/16743#comment:8
<p>
The last push only does one small thing really, the rest being to merge in other branches which have positive review or are merged. The actual change ("tweak") is to the comparison of ideals in number fields using the HNF: we were comparing the values of I.pari_hnf() which is a wrapped Pari GEN, and did not agree with what was intended, which comes from comparing I.pari_hnf().sage().
</p>
<p>
This is actually important for the purpose of ordering the primes in a number field, first by norm and then by their HNF. The observant reviewer may notice one doctest change in the prime_ideals() function, but this is a red herring caused by merging in the pari upgrade to 2.7.1 which gives a different generator for one ideal. For an actual change which the tweak causes you can look at the two primes of norm 17 in Q(i).
</p>
TicketjdemeyerWed, 24 Sep 2014 13:09:56 GMT
https://trac.sagemath.org/ticket/16743#comment:9
https://trac.sagemath.org/ticket/16743#comment:9
<p>
Sorry to bother, but it really would be much better to add the functions which have nothing to do with elliptic curves to the relevant places. For example, <code>prime_ideals()</code> should really be a method <code>prime_ideals_of_bounded_norm()</code> of <code>NumberField</code>. We don't want another <a class="closed ticket" href="https://trac.sagemath.org/ticket/11905" title="enhancement: Implement division_field() for elliptic curves (closed: fixed)">#11905</a>.
</p>
TicketcremonaWed, 24 Sep 2014 13:16:05 GMTstatus changed
https://trac.sagemath.org/ticket/16743#comment:10
https://trac.sagemath.org/ticket/16743#comment:10
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
</ul>
<p>
Good point, will do. It's clear for prime_ideals() and hnf_cmp() BUT in the latter case there is already a cmp function for ideals which is very similar but not good for my purposes: it does not first compare norms, and it uses the pari_hnf directly. So The simplest thing to do (replace the existing ideal cmp with mine) may cause a lot of annoying doctest changes. I'll try and see.
</p>
<p>
curve_cmp() can be a method of the class for elliptic curves over number fields, or even (almost) for generic elliptic curves as it uses no special number field stuff, just compares the list of a-invariants. Not quite since I replace each ai with its list of coefficients and flatten to get a list of 5*d rational numbers where d is the degree of the field.
</p>
<p>
curve_cmp_cm() is a bit of an experiment: for curves with (rational) CM only.
</p>
<p>
I really need these comparison functions for ordering of the elliptic curves in a single isogeny class in a deterministic way. For CM isogeny classes it is nicer to group together the curves whose Endomorphism ring is the same (they are all orders in the same imaginary quadratic field, but can have different indices in the maximal order). So curve_cmp_cm() is only really relevant in the context of isogeny classes. Comments?
</p>
<p>
Lastly there are a couple of utility functions concerning binary quadratic forms, which I will put elsewhere.
</p>
TicketcremonaWed, 24 Sep 2014 13:42:00 GMT
https://trac.sagemath.org/ticket/16743#comment:11
https://trac.sagemath.org/ticket/16743#comment:11
<p>
OK, so I would like some feedback on the change in ideal comparison. Quite a lot of doctests will need changing in sage/rings/number_field (I have not yet gone further), which looks bad BUT thee seem to be all due not to the addition of using a norm comparison first, but in the change from comparing pari_hfn() and pari_hnf().sage(), which in particular causes the primes in factorizations to be ordered differently. This looks like a big negative -- but see my earlier remark: I don't think we can trust the comparison of two Pari-GEN objects to mean what we think, so that although the old comparison appears to be comaring the HNFs of the ideals, it is not really doing that as far as I can see.
</p>
TicketjdemeyerWed, 24 Sep 2014 13:42:45 GMT
https://trac.sagemath.org/ticket/16743#comment:12
https://trac.sagemath.org/ticket/16743#comment:12
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/16743#comment:10" title="Comment 10">cremona</a>:
</p>
<blockquote class="citation">
<p>
Good point, will do. It's clear for prime_ideals() and hnf_cmp() BUT in the latter case there is already a cmp function for ideals which is very similar but not good for my purposes: it does not first compare norms, and it uses the pari_hnf directly. So The simplest thing to do (replace the existing ideal cmp with mine) may cause a lot of annoying doctest changes. I'll try and see.
</p>
</blockquote>
<p>
I don't think this should be the default <code>cmp()</code> for ideals (it's more expensive to compute), but it should be function defined in <code>rings/number_field/number_field_ideal.py</code>. And why do you need
</p>
<pre class="wiki">cmp(I.pari_hnf().sage(), J.pari_hnf().sage())
</pre><p>
as opposed to
</p>
<pre class="wiki">cmp(I.pari_hnf(), J.pari_hnf())
</pre><p>
That's not clear to me...
</p>
<p>
Finally, a Python tip: you can shorten
</p>
<pre class="wiki">t = int(I.norm() - J.norm())
if t:
return cmp(t,0)
return cmp(I.pari_hnf().sage(), J.pari_hnf().sage())
</pre><p>
to
</p>
<pre class="wiki">return cmp(I.norm(), J.norm()) or cmp(I.pari_hnf().sage(), J.pari_hnf().sage())
</pre>
TicketjdemeyerWed, 24 Sep 2014 13:44:53 GMT
https://trac.sagemath.org/ticket/16743#comment:13
https://trac.sagemath.org/ticket/16743#comment:13
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/16743#comment:11" title="Comment 11">cremona</a>:
</p>
<blockquote class="citation">
<p>
I don't think we can trust the comparison of two Pari-GEN objects to mean what we think
</p>
</blockquote>
<p>
What do you think that comparing HNFs should mean? I don't see a meaningful comparison, we just need it to be consistent.
</p>
TicketjdemeyerWed, 24 Sep 2014 13:49:45 GMT
https://trac.sagemath.org/ticket/16743#comment:14
https://trac.sagemath.org/ticket/16743#comment:14
<p>
Concerning comparisons, there is also <a class="closed ticket" href="https://trac.sagemath.org/ticket/16127" title="defect: Fix comparison of PARI objects (closed: fixed)">#16127</a> and <a class="closed ticket" href="https://trac.sagemath.org/ticket/17026" title="enhancement: Compare PARI objects using cmp_universal() instead of strcmp() (closed: fixed)">#17026</a>.
</p>
TicketcremonaWed, 24 Sep 2014 13:56:10 GMT
https://trac.sagemath.org/ticket/16743#comment:15
https://trac.sagemath.org/ticket/16743#comment:15
<p>
OK, so here is an example:
</p>
<pre class="wiki">sage: K.<i> = QuadraticField(-1)
sage: I = K.ideal(4+i)
sage: J = K.ideal(4-i)
sage: HI = I.pari_hnf()
sage: HJ = J.pari_hnf()
sage: HIS = HI.sage()
sage: HJS = HJ.sage()
sage: cmp(HI,HJ)
1
sage: cmp(HIS,HJS)
-1
</pre><p>
so they diagree. We have
</p>
<pre class="wiki">sage: HI, HJ
([17, 4; 0, 1], [17, 13; 0, 1])
sage: HIS, HJS
(
[17 4] [17 13]
[ 0 1], [ 0 1]
)
</pre><p>
and I think that I should come before J but using the pari_hnf as is gives the reverse.
</p>
<p>
Background: I am making databases of things like elliptic curves over number fields, and am having to be very explicit indeed about how various objects are ordered. This is a special case: two conjugate primes above a rational prime p which plsits in a quadratic field. I need to order these, and want to do so using the HNFs which only differ in one entry. Using pari_hnf is not consistent! (Try the ideals (2+i) and (2-i) and the pari_hnf's compare "correctly".)
</p>
<p>
I'll have to look at those two other tickets, certainly. Yet another quagmire!
</p>
TicketjdemeyerWed, 24 Sep 2014 14:05:24 GMT
https://trac.sagemath.org/ticket/16743#comment:16
https://trac.sagemath.org/ticket/16743#comment:16
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/16743#comment:12" title="Comment 12">jdemeyer</a>:
</p>
<blockquote class="citation">
<p>
Finally, a Python tip: you can shorten
</p>
<pre class="wiki">t = int(I.norm() - J.norm())
if t:
return cmp(t,0)
return cmp(I.pari_hnf().sage(), J.pari_hnf().sage())
</pre><p>
to
</p>
<pre class="wiki">return cmp(I.norm(), J.norm()) or cmp(I.pari_hnf().sage(), J.pari_hnf().sage())
</pre></blockquote>
<p>
Better yet: define a sorting <em>key</em> instead of a comparison function, this is the only(!) way in Python 3 and already works now. Your key would then be the tuple <code>(norm,hnf)</code>. See <a class="ext-link" href="http://python3porting.com/preparing.html#when-sorting-use-key-instead-of-cmp"><span class="icon"></span>http://python3porting.com/preparing.html#when-sorting-use-key-instead-of-cmp</a>
</p>
TicketjdemeyerWed, 24 Sep 2014 14:11:26 GMT
https://trac.sagemath.org/ticket/16743#comment:17
https://trac.sagemath.org/ticket/16743#comment:17
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/16743#comment:15" title="Comment 15">cremona</a>:
</p>
<blockquote class="citation">
<p>
I'll have to look at those two other tickets, certainly.
</p>
</blockquote>
<p>
I think that those will solve your problem, since matrices would be compared entry-wise, starting with the first column (top-left entry first, then the one below that).
</p>
TicketcremonaWed, 24 Sep 2014 14:17:18 GMT
https://trac.sagemath.org/ticket/16743#comment:18
https://trac.sagemath.org/ticket/16743#comment:18
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/16743#comment:17" title="Comment 17">jdemeyer</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/16743#comment:15" title="Comment 15">cremona</a>:
</p>
<blockquote class="citation">
<p>
I'll have to look at those two other tickets, certainly.
</p>
</blockquote>
<p>
I think that those will solve your problem, since matrices would be compared entry-wise, starting with the first column (top-left entry first, then the one below that).
</p>
</blockquote>
<p>
Right, so I have some work to do, and this ticket will acqure another dependency or two -- I hope those two tickets are not going to get held up!
</p>
TicketcremonaWed, 24 Sep 2014 14:46:11 GMT
https://trac.sagemath.org/ticket/16743#comment:19
https://trac.sagemath.org/ticket/16743#comment:19
<p>
Thanks a lot for your help and suggestions. I did not need that ideal comparison for this ticket anyway, so I have left alone the ideal comparison function except to tidy the code as you suggested. the two elliptic curve comparison functions have gone, replaced by little key comparison functions in the code where they are used -- much better and shorter and faster!
</p>
<p>
I am doing some final testing and then will upload a new branch in which you will find no utility functions in the wrong place.
</p>
<p>
In the other tickets where you are dealing with comparing pari objects, good luck: if you have not already discovered this, you will find that hunderds of number field factorizations have their orders changed....
</p>
TicketjdemeyerWed, 24 Sep 2014 15:11:58 GMT
https://trac.sagemath.org/ticket/16743#comment:20
https://trac.sagemath.org/ticket/16743#comment:20
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/16743#comment:15" title="Comment 15">cremona</a>:
</p>
<blockquote class="citation">
<p>
Background: I am making databases of things like elliptic curves over number fields, and am having to be very explicit indeed about how various objects are ordered.
</p>
</blockquote>
<p>
Before worrying about ideals, first consider the ring O_K. Since the HNF is essentially coordinates w.r.t. to a basis, the chosen Z-basis of O_K also matters a lot. For quadratic fields, one can easily define a canonical basis, but in higher degree that gets a lot more tricky.
</p>
TicketcremonaWed, 24 Sep 2014 15:38:13 GMT
https://trac.sagemath.org/ticket/16743#comment:21
https://trac.sagemath.org/ticket/16743#comment:21
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/16743#comment:20" title="Comment 20">jdemeyer</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/16743#comment:15" title="Comment 15">cremona</a>:
</p>
<blockquote class="citation">
<p>
Background: I am making databases of things like elliptic curves over number fields, and am having to be very explicit indeed about how various objects are ordered.
</p>
</blockquote>
<p>
Before worrying about ideals, first consider the ring O_K. Since the HNF is essentially coordinates w.r.t. to a basis, the chosen Z-basis of O_K also matters a lot. For quadratic fields, one can easily define a canonical basis, but in higher degree that gets a lot more tricky.
</p>
</blockquote>
<p>
Absolutely right! We have had alot of such discussions with the LMFDB project. Personally, I will stick to quadratic fields at least as far as making databases of elliptic curves is concerned (though the LMFDB does have some over the cubic field of discriminant -23).....
</p>
TicketgitWed, 24 Sep 2014 15:45:37 GMTcommit changed
https://trac.sagemath.org/ticket/16743#comment:22
https://trac.sagemath.org/ticket/16743#comment:22
<ul>
<li><strong>commit</strong>
changed from <em>06d9eb2226151a35bb2d3779e5309f4d066af8ca</em> to <em>c9bd24241141693a9984668f64a104cecdf7ebbd</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=c9bd24241141693a9984668f64a104cecdf7ebbd"><span class="icon"></span>c9bd242</a></td><td><code>#16743: moved or deleted various utility functions and simplified come sorting</code>
</td></tr></table>
TicketcremonaWed, 24 Sep 2014 15:47:46 GMT
https://trac.sagemath.org/ticket/16743#comment:23
https://trac.sagemath.org/ticket/16743#comment:23
<p>
I hope I have done what was wanted. On testing the whole library I find some doctest failures which I think were caused by merging the pari 2.7.1 upgrade branch, and I hope they will go away by themselves. I see that ticket <a class="closed ticket" href="https://trac.sagemath.org/ticket/15767" title="enhancement: Upgrade PARI to 2.7.1 (closed: fixed)">#15767</a> is merged, but I am not sure whether all of the relevant branch is already merged here.
</p>
TicketcremonaWed, 24 Sep 2014 15:48:00 GMTstatus changed
https://trac.sagemath.org/ticket/16743#comment:24
https://trac.sagemath.org/ticket/16743#comment:24
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
</ul>
TicketgitWed, 24 Sep 2014 16:33:14 GMTcommit changed
https://trac.sagemath.org/ticket/16743#comment:25
https://trac.sagemath.org/ticket/16743#comment:25
<ul>
<li><strong>commit</strong>
changed from <em>c9bd24241141693a9984668f64a104cecdf7ebbd</em> to <em>cfd3f54410084eaa0d051b8da6a618d8e65074b0</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=cfd3f54410084eaa0d051b8da6a618d8e65074b0"><span class="icon"></span>cfd3f54</a></td><td><code>#16743: corrected sorting in primes_of_bounded_norm</code>
</td></tr></table>
TicketcremonaWed, 24 Sep 2014 16:34:17 GMT
https://trac.sagemath.org/ticket/16743#comment:26
https://trac.sagemath.org/ticket/16743#comment:26
<p>
Oops, forgot to sort the primes using a dual key (norm,ideal). Note that the order will change after the comparison of pari gen objects changes.
</p>
TicketjdemeyerWed, 24 Sep 2014 18:53:49 GMTstatus changed
https://trac.sagemath.org/ticket/16743#comment:27
https://trac.sagemath.org/ticket/16743#comment:27
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
</ul>
<p>
Shaks -> Shanks
</p>
TicketjdemeyerWed, 24 Sep 2014 18:54:36 GMT
https://trac.sagemath.org/ticket/16743#comment:28
https://trac.sagemath.org/ticket/16743#comment:28
<p>
Hurwitx -> Hurwitz
</p>
TicketjdemeyerWed, 24 Sep 2014 19:00:45 GMT
https://trac.sagemath.org/ticket/16743#comment:29
https://trac.sagemath.org/ticket/16743#comment:29
<p>
In the implementation of <code>quadclassunit()</code>, using <code>0</code> for <code>prec</code> is wrong. Usually, <code>prec</code> arguments are set to <code>prec_bits_to_words(precision)</code> where <code>long precision=0</code> is the last argument of the Sage method. The <code>flag</code> is obsolete and <code>tech</code> should best be left alone, so I don't mind that you don't allow to set those.
</p>
TicketjdemeyerWed, 24 Sep 2014 19:04:30 GMT
https://trac.sagemath.org/ticket/16743#comment:30
https://trac.sagemath.org/ticket/16743#comment:30
<p>
The <code>EXAMPLES::</code> blocks in <code>BinaryQF.solve()</code> and in <code>small_prime_value()</code> are badly indented.
</p>
TicketjdemeyerWed, 24 Sep 2014 19:06:58 GMT
https://trac.sagemath.org/ticket/16743#comment:31
https://trac.sagemath.org/ticket/16743#comment:31
<p>
In this <code>solve()</code> method, I would prefer it made more clear that you're solving for integers (as opposed to, say, rationals). For example, replace the first line by
</p>
<pre class="wiki">Solve `Q(x,y) = n` in integers `x` and `y` where `Q` is this quadratic form.
</pre><p>
In the light of a future <code>solve()</code> for rationals, it might even be good to rename this method <code>solve_integer()</code>
</p>
TicketjdemeyerWed, 24 Sep 2014 19:15:22 GMT
https://trac.sagemath.org/ticket/16743#comment:32
https://trac.sagemath.org/ticket/16743#comment:32
<p>
Obvious failure:
</p>
<pre class="wiki">sage -t src/sage/rings/number_field/number_field.py
**********************************************************************
File "src/sage/rings/number_field/number_field.py", line 3073, in sage.rings.number_field.number_field.NumberField_generic.primes_of_bounded_norm
Failed example:
K.primes_of_bounded_norm(10)
Expected:
[Fractional ideal (i + 1), Fractional ideal (3), Fractional ideal (-i - 2), Fractional ideal (2*i + 1)]
Got:
[Fractional ideal (i + 1), Fractional ideal (-i - 2), Fractional ideal (2*i + 1), Fractional ideal (3)]
**********************************************************************
</pre>
TicketjdemeyerWed, 24 Sep 2014 19:16:21 GMT
https://trac.sagemath.org/ticket/16743#comment:33
https://trac.sagemath.org/ticket/16743#comment:33
<p>
You can simplify
</p>
<pre class="wiki">from sage.misc.all import flatten
P = flatten([pp for pp in [self.primes_above(p) for p in primes(B+1)]])
</pre><p>
by
</p>
<pre class="wiki">P = [pp for p in primes(B+1) for pp in self.primes_above(p)]
</pre><p>
(note the order of the <code>for</code> statements!)
</p>
TicketjdemeyerWed, 24 Sep 2014 19:21:14 GMT
https://trac.sagemath.org/ticket/16743#comment:34
https://trac.sagemath.org/ticket/16743#comment:34
<p>
In <code>primes_of_bounded_norm()</code>, why <code>B.ceil()</code>? This would disallow Python <code>int</code>s for no good reason. I think you can remove the whole block
</p>
<pre class="wiki">try:
B = ZZ(B.ceil())
except (TypeError, AttributeError):
raise TypeError("%s is not valid bound on prime ideals" % B)
</pre>
TicketjdemeyerWed, 24 Sep 2014 19:30:18 GMT
https://trac.sagemath.org/ticket/16743#comment:35
https://trac.sagemath.org/ticket/16743#comment:35
<p>
I'll make a reviewer patch with these changes...
</p>
TicketjdemeyerWed, 24 Sep 2014 20:23:46 GMTchangetime, branch, time changed
https://trac.sagemath.org/ticket/16743#comment:36
https://trac.sagemath.org/ticket/16743#comment:36
<ul>
<li><strong>changetime</strong>
changed from <em>09/24/14 19:30:18</em> to <em>09/24/14 19:30:18</em>
</li>
<li><strong>branch</strong>
changed from <em>u/cremona/ticket/16743</em> to <em>u/jdemeyer/ticket/16743</em>
</li>
<li><strong>time</strong>
changed from <em>07/30/14 13:20:05</em> to <em>07/30/14 13:20:05</em>
</li>
</ul>
TicketjdemeyerWed, 24 Sep 2014 20:24:55 GMTstatus, commit changed; reviewer set
https://trac.sagemath.org/ticket/16743#comment:37
https://trac.sagemath.org/ticket/16743#comment:37
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
<li><strong>commit</strong>
changed from <em>cfd3f54410084eaa0d051b8da6a618d8e65074b0</em> to <em>29d278401dd44cbdcdc7579d380dda0217dc4920</em>
</li>
<li><strong>reviewer</strong>
set to <em>Jeroen Demeyer</em>
</li>
</ul>
<p>
These are some reviewer fixes, however I do not intend to fully review this ticket.
</p>
<hr />
<p>
New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=29d278401dd44cbdcdc7579d380dda0217dc4920"><span class="icon"></span>29d2784</a></td><td><code>Various reviewer fixes</code>
</td></tr></table>
TicketcremonaThu, 25 Sep 2014 08:13:38 GMT
https://trac.sagemath.org/ticket/16743#comment:38
https://trac.sagemath.org/ticket/16743#comment:38
<p>
Thanks a lot for tidying various things up ( at least some of which were not introduced by me, I think!).
</p>
<p>
We do need someone with a good knowledge of isogenies to check this. In writing the code I had to work out quite a lot of mathematics for which I do not know a good source (in the CM and potential CM cases). I am actually using the code a lot all the time now, on many thousands of curves, which is a good test, but the field degrees are small.
</p>
<p>
I have merged your changes into my branch so if I make any further changes the branch name will go back to what it was (not that it really matters). I did this using
</p>
<pre class="wiki">git remote update
git merge trac/u/jdemeyer/ticket/16743
make
</pre><p>
(in my own local branch), for the record.
</p>
TicketjdemeyerThu, 25 Sep 2014 09:12:10 GMT
https://trac.sagemath.org/ticket/16743#comment:39
https://trac.sagemath.org/ticket/16743#comment:39
<p>
I usually use <code>./sage -dev</code> and then you can simply do <code>./sage -dev pull</code>.
</p>
TicketgitThu, 25 Sep 2014 09:13:26 GMTcommit changed
https://trac.sagemath.org/ticket/16743#comment:40
https://trac.sagemath.org/ticket/16743#comment:40
<ul>
<li><strong>commit</strong>
changed from <em>29d278401dd44cbdcdc7579d380dda0217dc4920</em> to <em>2d89c050dc89bc4dcb760e3558304c7c61cf119d</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=2d89c050dc89bc4dcb760e3558304c7c61cf119d"><span class="icon"></span>2d89c05</a></td><td><code>Add # long time is 2 places where it was needed</code>
</td></tr></table>
TicketcremonaThu, 25 Sep 2014 09:18:11 GMT
https://trac.sagemath.org/ticket/16743#comment:41
https://trac.sagemath.org/ticket/16743#comment:41
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/16743#comment:40" title="Comment 40">git</a>:
</p>
<blockquote class="citation">
<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=2d89c050dc89bc4dcb760e3558304c7c61cf119d"><span class="icon"></span>2d89c05</a></td><td><code>Add # long time is 2 places where it was needed</code>
</td></tr></table>
</blockquote>
<p>
It's pretty shocking that to take a list of 4 elliptic curves and compute their j-invariants and count the distinct ones takes a long time! Even if it is over a number field of degree 6. But so be it.
</p>
TicketjdemeyerThu, 25 Sep 2014 09:25:40 GMT
https://trac.sagemath.org/ticket/16743#comment:42
https://trac.sagemath.org/ticket/16743#comment:42
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/16743#comment:41" title="Comment 41">cremona</a>:
</p>
<blockquote class="citation">
<p>
It's pretty shocking that to take a list of 4 elliptic curves and compute their j-invariants and count the distinct ones takes a long time! Even if it is over a number field of degree 6. But so be it.
</p>
</blockquote>
<p>
No no, it is because those 2 tests depend on an earlier <code># long time</code> test. The following doesn't work:
</p>
<pre class="wiki">sage: x = answer_to_the_ultimate_question() # long time (7.5 million years)
sage: x
42
</pre><p>
If you run this without <code>--long</code> the first line will be skipped and the second line will therefore fail.
</p>
TicketcremonaThu, 25 Sep 2014 12:35:25 GMT
https://trac.sagemath.org/ticket/16743#comment:43
https://trac.sagemath.org/ticket/16743#comment:43
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/16743#comment:42" title="Comment 42">jdemeyer</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/16743#comment:41" title="Comment 41">cremona</a>:
</p>
<blockquote class="citation">
<p>
It's pretty shocking that to take a list of 4 elliptic curves and compute their j-invariants and count the distinct ones takes a long time! Even if it is over a number field of degree 6. But so be it.
</p>
</blockquote>
<p>
No no, it is because those 2 tests depend on an earlier <code># long time</code> test. The following doesn't work:
</p>
<pre class="wiki">sage: x = answer_to_the_ultimate_question() # long time (7.5 million years)
sage: x
42
</pre><p>
If you run this without <code>--long</code> the first line will be skipped and the second line will therefore fail.
</p>
</blockquote>
<p>
Of course, my stupidity.
</p>
TicketcremonaThu, 25 Sep 2014 12:37:53 GMT
https://trac.sagemath.org/ticket/16743#comment:44
https://trac.sagemath.org/ticket/16743#comment:44
<p>
Note to potential reviewers: Jeroen has reviewed the Sage-technical aspects of this, but we need someone to vouch for its mathematical correctness. For curves without CM there was little to do given the dependent tickets (now closed) which give the finite list of prime degrees to test, as the structure of the code already existed for curves over Q. The main thing which needs to be looked at is the CM case, where I had to work out several things myself, not knowing of suitable references.
</p>
TicketwuthrichThu, 25 Sep 2014 13:05:05 GMT
https://trac.sagemath.org/ticket/16743#comment:45
https://trac.sagemath.org/ticket/16743#comment:45
<p>
I spotted that one of the commits of <a class="closed ticket" href="https://trac.sagemath.org/ticket/16806" title="enhancement: Isogeny Bounds for Elliptic Curves over Number Fields (closed: fixed)">#16806</a> does not seem to be included in this :
</p>
<p>
<a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?h=8c9301671b354f7ba6c24d48f28d0e2b0698f39f"><span class="icon"></span>http://git.sagemath.org/sage.git/commit/?h=8c9301671b354f7ba6c24d48f28d0e2b0698f39f</a>
</p>
<p>
Not sure this is important, but it seems the right thing to include it if it is depending on that ticket.
</p>
<p>
I fear I won't have time to check the maths anytime soon. But I put it on my list of things to do.
</p>
TicketjdemeyerThu, 25 Sep 2014 13:09:34 GMT
https://trac.sagemath.org/ticket/16743#comment:46
https://trac.sagemath.org/ticket/16743#comment:46
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/16743#comment:44" title="Comment 44">cremona</a>:
</p>
<blockquote class="citation">
<p>
Note to potential reviewers: Jeroen has reviewed the Sage-technical aspects of this
</p>
</blockquote>
<p>
To be more precise, let's say I reviewed everything outside of <code>src/sage/schemes/elliptic_curves</code>
</p>
<p>
While skimming through the rest of the patch, I just noticed several <code># not tested</code> for the graphs. Any reason?
</p>
TicketcremonaThu, 25 Sep 2014 13:10:05 GMT
https://trac.sagemath.org/ticket/16743#comment:47
https://trac.sagemath.org/ticket/16743#comment:47
<p>
You are right, and while think the system would deal with this automatically (it's your addition to include new stuff in the ref manual, right?) I'll merge that in.
</p>
TicketcremonaThu, 25 Sep 2014 13:10:56 GMTcommit, branch changed
https://trac.sagemath.org/ticket/16743#comment:48
https://trac.sagemath.org/ticket/16743#comment:48
<ul>
<li><strong>commit</strong>
changed from <em>2d89c050dc89bc4dcb760e3558304c7c61cf119d</em> to <em>4d69051c2d2d9b1929ee1c58c3040f75b06804a6</em>
</li>
<li><strong>branch</strong>
changed from <em>u/jdemeyer/ticket/16743</em> to <em>u/cremona/ticket/16743</em>
</li>
</ul>
<p>
Sorry, my comment was in reply to Chris. I have merged in the commit he mentioned (and then switched back the branch of this ticket to my name, ha ha).
</p>
<p>
To answer Jeroen: I think I was under the impression that displaying graphs during testing would create problems, while it is nice to have the examples in the manual. If it is harmless then we can remove these tags.
</p>
TicketwuthrichThu, 25 Sep 2014 14:18:51 GMT
https://trac.sagemath.org/ticket/16743#comment:49
https://trac.sagemath.org/ticket/16743#comment:49
<p>
A quick look at the doc output shows quite a few problems in isogeny_class of elliptic curves over number fields.
</p>
TicketcremonaThu, 25 Sep 2014 14:31:03 GMTstatus changed
https://trac.sagemath.org/ticket/16743#comment:50
https://trac.sagemath.org/ticket/16743#comment:50
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
</ul>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/16743#comment:49" title="Comment 49">wuthrich</a>:
</p>
<blockquote class="citation">
<p>
A quick look at the doc output shows quite a few problems in isogeny_class of elliptic curves over number fields.
</p>
</blockquote>
<p>
Apologies. I will sort that out, no need for you to.
</p>
TicketjdemeyerThu, 25 Sep 2014 14:42:32 GMT
https://trac.sagemath.org/ticket/16743#comment:51
https://trac.sagemath.org/ticket/16743#comment:51
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/16743#comment:48" title="Comment 48">cremona</a>:
</p>
<blockquote class="citation">
<p>
I was under the impression that displaying graphs during testing would create problems, while it is nice to have the examples in the manual. If it is harmless then we can remove these tags.
</p>
</blockquote>
<p>
Please do! Graphics objects are doctested by plotting them to a temporary file. It's good to have those tests, since they check that plotting works (to some extent, it's not checked how the picture looks like).
</p>
<p>
Note that this plotting machinery might take a while, so it would be prudent to add <code># long time</code> to those plotting tests.
</p>
TicketgitThu, 25 Sep 2014 15:49:43 GMTcommit changed
https://trac.sagemath.org/ticket/16743#comment:52
https://trac.sagemath.org/ticket/16743#comment:52
<ul>
<li><strong>commit</strong>
changed from <em>4d69051c2d2d9b1929ee1c58c3040f75b06804a6</em> to <em>542460ddf2df2783fd1a9beb43922467c9a08097</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=542460ddf2df2783fd1a9beb43922467c9a08097"><span class="icon"></span>542460d</a></td><td><code>#16743: fix doc output problem and turn on isogeny graph tests</code>
</td></tr></table>
TicketcremonaThu, 25 Sep 2014 15:51:20 GMTstatus changed
https://trac.sagemath.org/ticket/16743#comment:53
https://trac.sagemath.org/ticket/16743#comment:53
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
</ul>
<p>
OK, I fixed the documentation display problem in ell_number_field (and a typo), and changed the "not tested" graph displays with "long time" which revealed 2 typos there which have been fixed (I had a matplotlib parameter as "edges_labels" twice.
</p>
<p>
Next!
</p>
<hr />
<p>
New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=542460ddf2df2783fd1a9beb43922467c9a08097"><span class="icon"></span>542460d</a></td><td><code>#16743: fix doc output problem and turn on isogeny graph tests</code>
</td></tr></table>
TicketgitMon, 29 Sep 2014 13:53:59 GMTcommit changed
https://trac.sagemath.org/ticket/16743#comment:54
https://trac.sagemath.org/ticket/16743#comment:54
<ul>
<li><strong>commit</strong>
changed from <em>542460ddf2df2783fd1a9beb43922467c9a08097</em> to <em>33da411070c800462911a248c7c9270649697d99</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=33da411070c800462911a248c7c9270649697d99"><span class="icon"></span>33da411</a></td><td><code>Merge branch 'develop' into isogs-ecnf</code>
</td></tr></table>
TicketcremonaTue, 28 Oct 2014 13:41:12 GMT
https://trac.sagemath.org/ticket/16743#comment:55
https://trac.sagemath.org/ticket/16743#comment:55
<p>
ping!
</p>
TicketwuthrichThu, 30 Oct 2014 12:05:38 GMTreviewer, branch, commit changed
https://trac.sagemath.org/ticket/16743#comment:56
https://trac.sagemath.org/ticket/16743#comment:56
<ul>
<li><strong>reviewer</strong>
changed from <em>Jeroen Demeyer</em> to <em>Jeroen Demeyer, Chris Wuthrich</em>
</li>
<li><strong>branch</strong>
changed from <em>u/cremona/ticket/16743</em> to <em>u/wuthrich/ticket/16743</em>
</li>
<li><strong>commit</strong>
changed from <em>33da411070c800462911a248c7c9270649697d99</em> to <em>79fee2d40805b8278b9d0afeaccf50eb101990da</em>
</li>
</ul>
<p>
pong!
</p>
<p>
I thought I would improve the documentation a little bit by including the isogeny class file into it. Other than that I can't see any reason to object against this ticket.
</p>
<p>
I am running the tests now.
</p>
<p>
As to the mathematics in here. Well, there is a lot and not all is trivial. I have read most of the new code, but I won't be able to verify everything. So my review is to confirm that John does very good work and that I have no doubts that it is as good as it can be.
</p>
<hr />
<p>
New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=a3cca91fba11797402fe60a98be7380edd6b7b22"><span class="icon"></span>a3cca91</a></td><td><code>Merge branch 'develop' into ticket/16743</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=79fee2d40805b8278b9d0afeaccf50eb101990da"><span class="icon"></span>79fee2d</a></td><td><code>trac 16743: changes to doc</code>
</td></tr></table>
TicketcremonaThu, 30 Oct 2014 12:28:58 GMT
https://trac.sagemath.org/ticket/16743#comment:57
https://trac.sagemath.org/ticket/16743#comment:57
<p>
Thanks, Chris. Of course there may well still be bugs, as always. The code was tested against completely independent determination of complete isogeny classes for a lot of curves over Q(sqrt(5)) and the cubic field of discriminant -23, and we find the same curves. That did not test the hardest part (namely the CM cases) though where I have done some systematic testing over imaginary quadratic fields of class number 1.
</p>
TicketwuthrichThu, 30 Oct 2014 17:25:20 GMTstatus changed
https://trac.sagemath.org/ticket/16743#comment:58
https://trac.sagemath.org/ticket/16743#comment:58
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
</ul>
<p>
The tests passed.
</p>
<p>
I also played a bit around. Seems all ok to me.
</p>
TicketvbraunSat, 15 Nov 2014 16:22:41 GMTstatus, branch changed; resolution set
https://trac.sagemath.org/ticket/16743#comment:59
https://trac.sagemath.org/ticket/16743#comment:59
<ul>
<li><strong>status</strong>
changed from <em>positive_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/wuthrich/ticket/16743</em> to <em>79fee2d40805b8278b9d0afeaccf50eb101990da</em>
</li>
</ul>
Ticket