Opened 11 years ago

Closed 11 years ago

# Bug in global_integral_model for elliptic curves over number fields

Reported by: Owned by: Johan Bosman John Cremona major sage-5.0 elliptic curves sage-5.0.beta9 Johan Bosman, John Cremona David Loeffler N/A

### Description

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]

### comment:1 Changed 11 years ago by John Cremona

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.

### comment:2 follow-up:  3 Changed 11 years ago by Johan Bosman

Authors: → Johan Bosman new → needs_review

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

### comment:3 in reply to:  2 Changed 11 years ago by John Cremona

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

### comment:4 Changed 11 years ago by David Loeffler

Authors: Johan Bosman → Johan Bosman, John Cremona → David Loeffler needs_review → positive_review

This looks fine to me.

### comment:5 Changed 11 years ago by Jeroen Demeyer

Merged in: → sage-5.0.beta9 → fixed positive_review → closed
Note: See TracTickets for help on using tickets.