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: | sage-7.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 5-isogenies of elliptic curves of j-invariant 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 follow-up: ↓ 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**2-2)/6
should be(beta**2-2*a)/6
.The bug was not caught by the doctests since the only example where this code was tested had a=1.