Opened 14 years ago

Closed 13 years ago

#5307 closed defect (fixed)

[with patch, positive review] Bug in conductor() over number fields

Reported by: John Cremona Owned by: William Stein
Priority: major Milestone: sage-4.1
Component: number theory Keywords: elliptic curve
Cc: Merged in: sage-4.1.alpha0
Authors: John Cremona Reviewers: Nick Alexander
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges


There is something wrong in the conductor computation of an elliptic curve over a field of class number >1:

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/ 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.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]
    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]

/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)

/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 == 1:

TypeError: _tidy_model() requires an integral model.

I think I wrote most of the relevant code, so it is my fault and I will fix it!

Attachments (1)

trac_5307.patch (8.8 KB) - added by John Cremona 13 years ago.

Download all attachments as: .zip

Change History (6)

comment:1 Changed 14 years ago by John Cremona

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 pi^i. 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 P^2; 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!

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.

I made a patch to implement this, and the example above works fine (doctest added to conductor() in

But 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.

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.

comment:2 Changed 14 years ago by Michael Abshoff

Milestone: sage-3.4sage-3.4.1

Better luck in 3.4.1.



Changed 13 years ago by John Cremona

Attachment: trac_5307.patch added

comment:3 Changed 13 years ago by John Cremona

Summary: Bug in conductor() over number fields[with patch, needs review] Bug in conductor() over number fields

This patch fixes the problem.

comment:4 Changed 13 years ago by ncalexan

Authors: John Cremona
Reviewers: Nick Alexander
Summary: [with patch, needs review] Bug in conductor() over number fields[with patch, positive review] Bug in conductor() over number fields

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!

comment:5 Changed 13 years ago by Robert Miller

Merged in: sage-4.1.alpha0
Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.