Opened 11 years ago

Closed 11 years ago

#11346 closed defect (fixed)

major bug in the conductor function for elliptic curves over number fields

Reported by: William Stein Owned by: John Cremona
Priority: critical Milestone: sage-4.7.1
Component: elliptic curves Keywords:
Cc: Merged in: sage-4.7.1.alpha2
Authors: William Stein Reviewers: Robert Bradshaw
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by William Stein)

Joanna Gaski found a serious bug in the function for computing conductors of elliptic curves over number fields, when the input curve is not integral. Witness:

sage: K.<g> = NumberField(x^2 - x - 1)
sage: E1 = EllipticCurve(K,[0,0,0,-1/48,-161/864]); E1
Elliptic Curve defined by y^2 = x^3 + (-1/48)*x + (-161/864) over Number Field in g with defining polynomial x^2 - x - 1
sage: factor(E1.conductor())
(Fractional ideal (3)) * (Fractional ideal (-2*g + 1))
sage: factor(E1.integral_model().conductor())
(Fractional ideal (2))^4 * (Fractional ideal (3)) * (Fractional ideal (-2*g + 1))

The bug is actually in the local_data() function, which computes the possible primes of bad reduction by taking the support of the discriminant. However, this is simply wrong if the input curve is not integral.

sage: E1.discriminant().support()
[Fractional ideal (-2*g + 1), Fractional ideal (3)]
sage: E1.integral_model().discriminant().support()
[Fractional ideal (-2*g + 1), Fractional ideal (2), Fractional ideal (3)]

The one-line fix is to first compute an integral model, then ask for the discriminant of that model in the local_data function.

Attachments (2)

trac_11346.patch (1.4 KB) - added by William Stein 11 years ago.
11346-followup.patch (1.3 KB) - added by Robert Bradshaw 11 years ago.

Download all attachments as: .zip

Change History (8)

comment:1 Changed 11 years ago by William Stein

Description: modified (diff)

Changed 11 years ago by William Stein

Attachment: trac_11346.patch added

comment:2 Changed 11 years ago by William Stein

Status: newneeds_review

comment:3 Changed 11 years ago by Robert Bradshaw

Status: needs_reviewpositive_review

comment:4 Changed 11 years ago by John Cremona

Authors: William Stein
Reviewers: Robert Bradshaw

Additional comment on this and #11347. I think a better fix would have been to add one line in the constructor for EllipticCurveLocalData in sage.schemes.elliptic_curves.ell_local_data, namely

    E = E.integral_model()

rather than having a lot of separate functions have to remember to do this. The fixes here and at #11347 are fine by themselves, but it remains the case that

sage: K.<g> = NumberField(x^2 - x - 1)          
sage: E = EllipticCurve(K,[0,0,0,-1/48,161/864])
sage: E.local_data()   

fails.

I don't have time to rework this right now, so have not tagged this "needs work"...

Changed 11 years ago by Robert Bradshaw

Attachment: 11346-followup.patch added

comment:5 Changed 11 years ago by Robert Bradshaw

I agree, followup patch does just that.

comment:6 Changed 11 years ago by Jeroen Demeyer

Merged in: sage-4.7.1.alpha2
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.