Sage: Ticket #20737: Use of representative_prime may fail in finding semi-global minimal models of elliptic curves
https://trac.sagemath.org/ticket/20737
<p>
When an elliptic curve E defined over a number field has no global minimal model, the obstruction is a non-trivial ideal class and E.global_minimal_model(semi_global=True) finds a prime ideal in that class and returns a model minimal at all primes but that one. However the line in kraus.py (line 916) does that using {{{P = c.representative_prime()}} which can fail since by default only primes of norm up to 1000 are tested, as that is the default value of the parameter norm_bound of representative_prime().
</p>
<p>
Example (an elliptic curve with everywhere good reduction over an imaginary quadratic field of class number 3315):
</p>
<pre class="wiki">sage: K.<a> = NumberField(x^2-x+31821453)
sage: ainvs = (0, 0, 0, -382586771000351226384*a - 2498023791133552294513515, 358777608829102441023422458989
744*a + 1110881475104109582383304709231832166)
sage: E = EllipticCurve(ainvs)
sage: assert E.conductor().norm() == 1
sage: E.global_minimal_model(semi_global=True)
<boom>
RuntimeError: No prime of norm less than 1000 found in class Fractional ideal class (1569, a + 867)
</pre><p>
Experiment with this example shows that the smallest prime in that class has norm 23567.
</p>
<p>
The obvious solution is to keep doubling the bound until success. The ideal in the error message has norm 1569 and is a product of two primes so could be used instead but then the model returned would not be "semi-global" in the above sense.
</p>
<p>
Whatever is decided, the semi-global=True option (which is not the default) should not cause a run-time error.
</p>
en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/20737
Trac 1.1.6cremonaMon, 24 Oct 2016 08:46:42 GMTstatus, milestone changed; commit, author, branch set
https://trac.sagemath.org/ticket/20737#comment:1
https://trac.sagemath.org/ticket/20737#comment:1
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>needs_review</em>
</li>
<li><strong>commit</strong>
set to <em>0d771e0dbdf553fa17e2f6b432acf349ada80d8d</em>
</li>
<li><strong>author</strong>
set to <em>John Cremona</em>
</li>
<li><strong>branch</strong>
set to <em>u/cremona/20737</em>
</li>
<li><strong>milestone</strong>
changed from <em>sage-7.3</em> to <em>sage-7.5</em>
</li>
</ul>
<p>
This fixes the problem by calling the representative_prime function with an explicit bound and repeatedly increaisng the bound until success. It might be preferable to rewrite the representative_prime method on ideal classes to allow (in effect) bound=Infinity, but that can be done elsewhere.
</p>
<p>
I added a new test with the example given here, but it takes 15s to find the prime so the test is tagged with # long time.
</p>
<hr />
<p>
New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit?id=0d771e0dbdf553fa17e2f6b432acf349ada80d8d"><span class="icon"></span>0d771e0</a></td><td><code>#20737: fix bug causing failure in semi-global minimal models of elliptic curves</code>
</td></tr></table>
TicketcremonaWed, 26 Oct 2016 19:42:39 GMT
https://trac.sagemath.org/ticket/20737#comment:2
https://trac.sagemath.org/ticket/20737#comment:2
<p>
There are currently 2 patchbot report for this ticket: one says that all tests pass but that some plugin gailed (what's that?). the other says that a test failed but this is bogus, just some file being tested timed out for reasons nothing to do with this ticket.
</p>
<p>
But I am guessing that the negative reports will prevent a uman with some intelligence looking at this. Not good!
</p>
TicketwuthrichSun, 06 Nov 2016 01:14:27 GMTstatus changed; reviewer set
https://trac.sagemath.org/ticket/20737#comment:3
https://trac.sagemath.org/ticket/20737#comment:3
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
<li><strong>reviewer</strong>
set to <em>Chris Wuthrich</em>
</li>
</ul>
<p>
All fine. Tests pass and even the patchbot is fully happy.
</p>
<p>
The new test is a bit long, but I can't see a way to make that faster and I agree that a test for this should be added.
</p>
TicketvbraunMon, 07 Nov 2016 18:27:17 GMTstatus, branch changed; resolution set
https://trac.sagemath.org/ticket/20737#comment:4
https://trac.sagemath.org/ticket/20737#comment:4
<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/cremona/20737</em> to <em>0d771e0dbdf553fa17e2f6b432acf349ada80d8d</em>
</li>
</ul>
Ticket