Opened 12 years ago

Closed 11 years ago

#5307 closed defect (fixed)

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

Reported by: cremona Owned by: was
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: Work issues:
Branch: Commit:
Dependencies: Stopgaps:


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 cremona 11 years ago.

Download all attachments as: .zip

Change History (6)

comment:1 Changed 12 years ago by 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 12 years ago by mabshoff

  • Milestone changed from sage-3.4 to sage-3.4.1

Better luck in 3.4.1.



Changed 11 years ago by cremona

comment:3 Changed 11 years ago by cremona

  • Summary changed from Bug in conductor() over number fields to [with patch, needs review] Bug in conductor() over number fields

This patch fixes the problem.

comment:4 Changed 11 years ago by ncalexan

  • Authors set to John Cremona
  • Reviewers set to Nick Alexander
  • Summary changed from [with patch, needs review] Bug in conductor() over number fields to [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 11 years ago by rlm

  • Merged in set to sage-4.1.alpha0
  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.