#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: |
Description (last modified by )
John Cremona noted that
sage: EllipticCurve('50700u1').galois_representation().image_type(13)
produces
AssertionError: bug in image_type.
Apply
Attachments (2)
Change History (17)
comment:1 Changed 10 years ago by
Component: | PLEASE CHANGE → elliptic curves |
---|---|
Owner: | changed from tbd to John Cremona |
Changed 10 years ago by
Attachment: | trac_14577_bug_in_galois_reps.patch added |
---|
comment:2 Changed 10 years ago by
Status: | new → needs_review |
---|
comment:3 Changed 10 years ago by
Authors: | → Chris Wuthrich |
---|
comment:4 follow-up: 5 Changed 10 years ago by
Status: | needs_review → needs_work |
---|
comment:5 Changed 10 years ago by
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).
comment:6 Changed 10 years ago by
Status: | needs_work → needs_review |
---|
ok, I have done that.
Otherwise, one would have to make the test optional.
Changed 10 years ago by
Attachment: | trac_14577_bug_in_galois_reps-addon1.patch added |
---|
comment:7 Changed 10 years ago by
Thanks for the patch. Can you give a positive review for the combination ?
comment:8 Changed 10 years ago by
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
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
Description: | modified (diff) |
---|
comment:11 Changed 10 years ago by
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.
comment:12 follow-up: 13 Changed 10 years ago by
Status: | needs_review → positive_review |
---|
ok, then positive review.
comment:13 Changed 10 years ago by
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
Merged in: | → sage-5.10.rc0 |
---|---|
Resolution: | → fixed |
Status: | positive_review → closed |
comment:15 Changed 9 years ago by
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.
I have uploaded an add-on patch which
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.