Sage: Ticket #5307: [with patch, positive review] Bug in conductor() over number fields
https://trac.sagemath.org/ticket/5307
<p>
There is something wrong in the conductor computation of an elliptic curve over a field of class number >1:
</p>
<pre class="wiki">sage: K.<w>=NumberField(x^2+x+6)
sage: E=EllipticCurve([w,-1,0,-w-6,0])
sage: E.conductor()
---------------------------------------------------------------------------
TypeError Traceback (most recent call last)
/home/masgaj/.sage/temp/host_56_150/7547/_home_masgaj__sage_init_sage_0.py in <module>()
/local/jec/sage-3.3.rc0/local/lib/python2.5/site-packages/sage/schemes/elliptic_curves/ell_number_field.pyc in conductor(self)
745 OK = self.base_ring().ring_of_integers()
746 self._conductor = prod([d.prime()**(d.conductor_valuation()) \
--> 747 for d in self.local_data()],\
748 OK.ideal(1))
749 return self._conductor
/local/jec/sage-3.3.rc0/local/lib/python2.5/site-packages/sage/schemes/elliptic_curves/ell_number_field.pyc in local_data(self, P, proof)
394 if P is None:
395 primes = self.base_ring()(self.discriminant()).support()
--> 396 return [self._get_local_data(pr, proof) for pr in primes]
397
398 from sage.schemes.elliptic_curves.ell_local_data import check_prime
/local/jec/sage-3.3.rc0/local/lib/python2.5/site-packages/sage/schemes/elliptic_curves/ell_number_field.pyc in _get_local_data(self, P, proof)
416 pass
417 from sage.schemes.elliptic_curves.ell_local_data import EllipticCurveLocalData
--> 418 self._local_data[P] = EllipticCurveLocalData(self, P, proof)
419 return self._local_data[P]
420
/local/jec/sage-3.3.rc0/local/lib/python2.5/site-packages/sage/schemes/elliptic_curves/ell_local_data.pyc in __init__(self, E, P, proof, algorithm)
138 self._reduction_type = Eint.ap(p) # = 0,-1 or +1
139 else:
--> 140 self._Emin, ch, self._val_disc, self._fp, self._KS, self._cp, self._split = self._tate(proof)
141 if self._fp>0:
142 if self._Emin.c4().valuation(p)>0:
/local/jec/sage-3.3.rc0/local/lib/python2.5/site-packages/sage/schemes/elliptic_curves/ell_local_data.pyc in _tate(self, proof)
748 a6 /= pi**6
749 verbose("Non-minimal equation, dividing out...\nNew model is %s"%([a1, a2, a3, a4, a6]), t, 1)
--> 750 C = C._tidy_model()
751 return (C, p, val_disc, fp, KS, cp, split)
752
/local/jec/sage-3.3.rc0/local/lib/python2.5/site-packages/sage/schemes/elliptic_curves/ell_number_field.pyc in _tidy_model(self)
297 (a1, a2, a3, a4, a6) = [ZK(a) for a in self.a_invariants()]
298 except TypeError:
--> 299 raise TypeError, "_tidy_model() requires an integral model."
300 # N.B. Must define s, r, t in the right order.
301 if ZK.degree() == 1:
TypeError: _tidy_model() requires an integral model.
</pre><p>
I think I wrote most of the relevant code, so it is my fault and I will fix it!
</p>
en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/5307
Trac 1.2John CremonaWed, 18 Feb 2009 22:42:21 GMT
https://trac.sagemath.org/ticket/5307#comment:1
https://trac.sagemath.org/ticket/5307#comment:1
<p>
Diagnosis of the problem, while lies in the implementation of Tate's algorithm at a prime ideal P when P is not principal: we use a uniformiser pi of P, but we use it in two different ways. First, there are various places where integers (of the field) which are known to have valuation at least i are divided by <code>pi^i</code>. Here, in order to keep everything integral, we use a uniformizer computed via K.uniformizer(P, 'negative'), which has valuation 1 at P (of course) and nagative or zero valuation elsewhere. But there is a second way in which pi is used: in computing the appropriate [u,r,s,t]-transforms. For example, in one place we need an r-transform where r is 0 mod P but something specific mod <code>P^2</code>; so we write r=r0*pi and compute r0 mod P and then multiply by pi. But now, it matters if pi is not integral!
</p>
<p>
The solution I came up with was to compute two uniformisers, one (pi) as above and another (called ipi) integral at all primes. I use the appropriate one in the appropriate places.
</p>
<p>
I made a patch to implement this, and the example above works fine (doctest added to conductor() in ell_number_field.py).
</p>
<p>
<span class="underline">But</span> I think this needs to be looked at more carefully; while it is no worse than before (and no different at all at principal primes) I don't think it is quite right yet.
</p>
<p>
NB Magma has essentially the same code (I wrote it) but is not fussy about integrality at all since it does not give local minimal models.
</p>
TicketMichael AbshoffSun, 01 Mar 2009 02:26:51 GMTmilestone changed
https://trac.sagemath.org/ticket/5307#comment:2
https://trac.sagemath.org/ticket/5307#comment:2
<ul>
<li><strong>milestone</strong>
changed from <em>sage-3.4</em> to <em>sage-3.4.1</em>
</li>
</ul>
<p>
Better luck in 3.4.1.
</p>
<p>
Cheers,
</p>
<p>
Michael
</p>
TicketJohn CremonaMon, 15 Jun 2009 19:30:44 GMTattachment set
https://trac.sagemath.org/ticket/5307
https://trac.sagemath.org/ticket/5307
<ul>
<li><strong>attachment</strong>
set to <em>trac_5307.patch</em>
</li>
</ul>
TicketJohn CremonaMon, 15 Jun 2009 19:31:54 GMTsummary changed
https://trac.sagemath.org/ticket/5307#comment:3
https://trac.sagemath.org/ticket/5307#comment:3
<ul>
<li><strong>summary</strong>
changed from <em>Bug in conductor() over number fields</em> to <em>[with patch, needs review] Bug in conductor() over number fields</em>
</li>
</ul>
<p>
This patch fixes the problem.
</p>
TicketncalexanMon, 15 Jun 2009 20:44:28 GMTsummary changed; reviewer, author set
https://trac.sagemath.org/ticket/5307#comment:4
https://trac.sagemath.org/ticket/5307#comment:4
<ul>
<li><strong>reviewer</strong>
set to <em>Nick Alexander</em>
</li>
<li><strong>author</strong>
set to <em>John Cremona</em>
</li>
<li><strong>summary</strong>
changed from <em>[with patch, needs review] Bug in conductor() over number fields</em> to <em>[with patch, positive review] Bug in conductor() over number fields</em>
</li>
</ul>
<p>
I can't say for sure that the new algorithm is correct, but I have read the code and I believe that the patch produces the behaviour John documented in the ticket and the comments. It certainly fixes the presenting issue, so apply!
</p>
TicketRobert MillerWed, 24 Jun 2009 09:50:45 GMTstatus changed; resolution, merged set
https://trac.sagemath.org/ticket/5307#comment:5
https://trac.sagemath.org/ticket/5307#comment:5
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>closed</em>
</li>
<li><strong>resolution</strong>
set to <em>fixed</em>
</li>
<li><strong>merged</strong>
set to <em>sage-4.1.alpha0</em>
</li>
</ul>
Ticket