Opened 6 years ago
Closed 6 years ago
#19840 closed defect (fixed)
Bug in elliptic curve isogeny
Reported by:  cremona  Owned by:  

Priority:  major  Milestone:  sage7.0 
Component:  elliptic curves  Keywords:  isogenies 
Cc:  Merged in:  
Authors:  John Cremona  Reviewers:  Frédéric Chapoton 
Report Upstream:  N/A  Work issues:  
Branch:  8ce0e65 (Commits, GitHub, GitLab)  Commit:  8ce0e65fd331eab23df923b43e31f3589c4d008f 
Dependencies:  #19689  Stopgaps: 
Description
There is a bug in the code to compute 5isogenies of elliptic curves of jinvariant 1728, when 5 is a square.
sage: K.<a> = NumberField(x^4  5*x^2 + 5) sage: E = EllipticCurve([a^2 + a + 1, a^3 + a^2 + a + 1, a^2 + a, 17*a^3 + 34*a^2  16*a  37, 54*a^3 + 105*a^2  66*a  135]) sage: E.j_invariant() 1728 sage: K(5).is_square() True sage: E.isogenies_prime_degree(5) ValueError: The polynomial does not define a finite subgroup of the elliptic curve.
or more directly
sage: from sage.schemes.elliptic_curves.isogeny_small_degree import isogenies_5_1728 sage: isogenies_5_1728(E) ValueError: The polynomial does not define a finite subgroup of the elliptic curve.
I wrote this code about 5 years ago, and will fix it.
Change History (15)
comment:1 Changed 6 years ago by
comment:2 Changed 6 years ago by
 Branch set to u/cremona/19840
 Commit set to a88c9a582fb4c487fb1275781ae0a667acc3527e
 Status changed from new to needs_review
To review, run the code in the description before and after; note that a doctest is added with this example.
New commits:
a88c9a5  #19840: isogeny bug fix and test

comment:3 Changed 6 years ago by
There is a small merge conflict with #19689 so I will rebase this on that and make that ticket a dependency.
comment:4 Changed 6 years ago by
 Commit changed from a88c9a582fb4c487fb1275781ae0a667acc3527e to 80595c1462255358891273fa2b3e26e2fa22f0aa
comment:5 Changed 6 years ago by
 Dependencies set to #19689
comment:6 followup: ↓ 7 Changed 6 years ago by
two details:
 The following is not formatted correctly
See `trac``19840`:
but should be
See :trac:`19840`:
 A typo in the first line: "with repect"
comment:7 in reply to: ↑ 6 Changed 6 years ago by
Replying to chapoton:
two details:
 The following is not formatted correctly
See `trac``19840`:but should be
See :trac:`19840`:
 A typo in the first line: "with repect"
Thanks, I am fixing those now and hope we can this into 7.0.
comment:8 Changed 6 years ago by
 Commit changed from 80595c1462255358891273fa2b3e26e2fa22f0aa to e6ed40dbc1b229e9966c05a926b1b9bb043a9502
Branch pushed to git repo; I updated commit sha1. New commits:
e6ed40d  #19840: correct typos after review

comment:9 Changed 6 years ago by
Don't forget to fill in the "reviewer" box :)
comment:10 Changed 6 years ago by
 Reviewers set to Frédéric Chapoton
sorry, still not formatted correctly, should be in fact
See :trac:`19840`::
with a double colon at the end.
Once done, you can set a positive review.
comment:11 Changed 6 years ago by
Sorry, will do. It is so hard to actually test correct formatting of docstrings since it takes a very long time to build doc and the output is thousamds of lines somewhere in which is the relevant line or two.
comment:12 Changed 6 years ago by
 Commit changed from e6ed40dbc1b229e9966c05a926b1b9bb043a9502 to 8ce0e65fd331eab23df923b43e31f3589c4d008f
Branch pushed to git repo; I updated commit sha1. New commits:
8ce0e65  #19840 last typo

comment:13 Changed 6 years ago by
 Status changed from needs_review to positive_review
comment:14 Changed 6 years ago by
7.0 please!
comment:15 Changed 6 years ago by
 Branch changed from u/cremona/19840 to 8ce0e65fd331eab23df923b43e31f3589c4d008f
 Resolution set to fixed
 Status changed from positive_review to closed
I found the bug: on line 868,
a*(beta**22)/6
should be(beta**22*a)/6
.The bug was not caught by the doctests since the only example where this code was tested had a=1.