Opened 10 years ago

Closed 10 years ago

Last modified 9 years ago

#14577 closed defect (fixed)

Bug in the case S_4 in Galois representations of elliptic curves

Reported by: wuthrich Owned by: John Cremona
Priority: major Milestone: sage-5.10
Component: elliptic curves Keywords: galois representations, elliptic curves
Cc: Merged in: sage-5.10.rc0
Authors: Chris Wuthrich Reviewers: Frédéric Chapoton
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by Frédéric Chapoton)

John Cremona noted that

sage: EllipticCurve('50700u1').galois_representation().image_type(13)

produces

AssertionError: bug in image_type.

Apply

Attachments (2)

trac_14577_bug_in_galois_reps.patch (1.5 KB) - added by wuthrich 10 years ago.
trac_14577_bug_in_galois_reps-addon1.patch (7.1 KB) - added by Frédéric Chapoton 10 years ago.

Download all attachments as: .zip

Change History (17)

comment:1 Changed 10 years ago by wuthrich

Component: PLEASE CHANGEelliptic curves
Owner: changed from tbd to John Cremona

Changed 10 years ago by wuthrich

comment:2 Changed 10 years ago by wuthrich

Status: newneeds_review

comment:3 Changed 10 years ago by wuthrich

Authors: Chris Wuthrich

comment:4 Changed 10 years ago by Frédéric Chapoton

Status: needs_reviewneeds_work

I have uploaded an add-on patch which

  • puts the raise statements into python3 syntax
  • adds the links to the trac tickets

But there is a problem with your patch: one doctest is failing, there is no curve called '50700u1', even after installing the extended Cremona database.

comment:5 in reply to:  4 Changed 10 years ago by John Cremona

Replying to chapoton:

I have uploaded an add-on patch which

  • puts the raise statements into python3 syntax
  • adds the links to the trac tickets

But there is a problem with your patch: one doctest is failing, there is no curve called '50700u1', even after installing the extended Cremona database.

?? Yes there is! However, in order for this test to be usable without the optional database I suggest replacing it with

EllipticCurve([0,1,0,-4788,109188]).galois_representation().image_type(13)

(I have inserted the a-invariants of 50700u1).

Last edited 10 years ago by John Cremona (previous) (diff)

comment:6 Changed 10 years ago by Frédéric Chapoton

Status: needs_workneeds_review

ok, I have done that.

Otherwise, one would have to make the test optional.

Changed 10 years ago by Frédéric Chapoton

comment:7 Changed 10 years ago by wuthrich

Thanks for the patch. Can you give a positive review for the combination ?

comment:8 Changed 10 years ago by Frédéric Chapoton

Reviewers: Frédéric Chapoton

Sorry, but I am not able to check the math. If you are sure of the math, you can put a positive review in my name.

comment:9 Changed 10 years ago by John Cremona

Chris, can you give us (here) a quick note about the new el5 assignment from roots of a quadratic instead of whatever it was before?

Also, we need to put instructions in the descriptions about applying both patches in sequence.

comment:10 Changed 10 years ago by Frédéric Chapoton

Description: modified (diff)

comment:11 Changed 10 years ago by wuthrich

Yes, I am sure this improves this function.

The function _ex_set(p) at the start of the file lists the elements in PGL_2(F_p) that may appear in an exceptional subgroup A_n or S_n. Or rather it lists what trace^2/det evaluates on them. (This is taken from Serre's paper). In particular the elements of order 5 are the roots of (X**2 - 3*X+1), which we add at the end to the list.

Now later this is used in the function image_type. Unfortunately, I took the last two elements, when in fact for some p there is are no such elements of order 5. As a consequence, the example that John found all possibilities for what the subgroup could be were eliminated. Hence the answer "bug". Now it gives the correct answer.

So the maths did not change, just a silly bug was eliminated.

Last edited 10 years ago by wuthrich (previous) (diff)

comment:12 Changed 10 years ago by Frédéric Chapoton

Status: needs_reviewpositive_review

ok, then positive review.

comment:13 in reply to:  12 Changed 10 years ago by John Cremona

Replying to chapoton:

ok, then positive review.

Seconded -- thanks for the explanation. I think it is better to have that written on this ticket, though I know I could have worked that out for myself, if it had not been the week of exam marking for me...

comment:14 Changed 10 years ago by Jeroen Demeyer

Merged in: sage-5.10.rc0
Resolution: fixed
Status: positive_reviewclosed

comment:15 Changed 9 years ago by John Cremona

Too late for this ticket, but unfortunately a typo causes this:

EllipticCurve([0, 0, 0, -1129345880,-86028258620304]).galois_representation().image_type(11)

gives

NameError: global name 'nonsplit_str' is not defined

because the identifier non_split_str is referred to wrongly on lines 1054 and 1108 in gal_reps.py. I have another patch which I will put onto a new ticket: #14752.

Last edited 9 years ago by John Cremona (previous) (diff)
Note: See TracTickets for help on using tickets.