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:

Status badges

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 cremona

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.

comment:2 Changed 6 years ago by cremona

  • 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 cremona

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 git

  • Commit changed from a88c9a582fb4c487fb1275781ae0a667acc3527e to 80595c1462255358891273fa2b3e26e2fa22f0aa

Branch pushed to git repo; I updated commit sha1. New commits:

ff0ba0c#19689: unit scaling for Weierstrass models of elliptic curves over number fields
80595c1#19840: isogeny bug fix and test

comment:5 Changed 6 years ago by cremona

  • Dependencies set to #19689

comment:6 follow-up: Changed 6 years ago by 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"

comment:7 in reply to: ↑ 6 Changed 6 years ago by cremona

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 git

  • 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 cremona

Don't forget to fill in the "reviewer" box :)

comment:10 Changed 6 years ago by chapoton

  • 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 cremona

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 git

  • 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 cremona

  • Status changed from needs_review to positive_review

comment:14 Changed 6 years ago by cremona

7.0 please!

comment:15 Changed 6 years ago by vbraun

  • Branch changed from u/cremona/19840 to 8ce0e65fd331eab23df923b43e31f3589c4d008f
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.