Bug in global_integral_model for elliptic curves over number fields

[sagetrac-johanbosman]

sage: K.<v> = NumberField(x^2 + 161*x - 150)
sage: E = EllipticCurve([25105/216*v - 3839/36, 634768555/7776*v - 98002625/1296, 634768555/7776*v - 98002625/1296, 0, 0])
sage: E.global_integral_model()
...
AssertionError: bug in global_integral_model: [-511235417/8*v + 238969025/4, 789861012140869185/32*v - 365842578320087625/16, -434331620876169353603835/32*v + 201170993209979865073875/16, 0, 0]

Component: elliptic curves

Author: Johan Bosman, John Cremona

Reviewer: David Loeffler

Merged: sage-5.0.beta9

Issue created by migration from https://trac.sagemath.org/ticket/12151

[JohnCremona]

comment:1

This can be fixed by changing

for P, _ in K.ideal(a.denominator()).factor():

on line 564 of ell_number_field.py to

for P in [P for P,e in K.ideal(a).factor() if e<0]:

or alternatively (I think)

for P, _ in a.denominator_ideal().factor():

I checked that the first alternative works.

NB I also think that the line

pi = K.uniformizer(P,'positive')

should be

pi = K.uniformizer(P,'negative')

since we will divide by a power of pi and want to make sure that the model stays integral at other primes. This does not matter in the example given where the class number is 1 so each pi will be an actual generator of the prime ideal.

[sagetrac-johanbosman]

Attachment: 12151.patch.gz

[sagetrac-johanbosman]

Author: Johan Bosman

[sagetrac-johanbosman]

comment:2

Changing negative into positive was done in #7935, so I've decided to keep it positive. ;).

[JohnCremona]

comment:3

Replying to @sagetrac-johanbosman:

Changing negative into positive was done in #7935, so I've decided to keep it positive. ;).

I have CC'd Chris Wuthrich who made the patch at #7935 (where I made a comment on exactly that line).

[loefflerd]

Reviewer: David Loeffler

[loefflerd]

Changed author from Johan Bosman to Johan Bosman, John Cremona

[loefflerd]

comment:4

This looks fine to me.

[jdemeyer]

Merged: sage-5.0.beta9