Opened 11 years ago

Closed 11 years ago

#8349 closed defect (fixed)

bug in isogenies

Reported by: wuthrich Owned by: cremona
Priority: minor Milestone: sage-4.3.4
Component: elliptic curves Keywords: isogeny, elliptic curves
Cc: Merged in: sage-4.3.4.alpha0
Authors: Craig Citro, Chris Wuthrich Reviewers: John Cremona
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

Something is wrong with the post_isomorphism of isogenies of elliptic curves :

sage: E = EllipticCurve(GF(17), [0,-1,0,-3,-1])
sage: P = E([16,0])
sage: phi = E.isogeny(P,codomain=E)
sage: phi(P)
(9 : 11 : 1)
sage: phi(P) in E
False

Attachments (2)

trac_8349.patch (1.2 KB) - added by craigcitro 11 years ago.
trac_8349.2.patch (1.5 KB) - added by wuthrich 11 years ago.

Download all attachments as: .zip

Change History (10)

Changed 11 years ago by craigcitro

comment:1 Changed 11 years ago by craigcitro

  • Status changed from new to needs_review

Attached a quick fix -- I'm happy to let it be ignored if there's something classier to be done.

comment:2 Changed 11 years ago by wuthrich

  • Status changed from needs_review to needs_work

Wow. That was very quick. But maybe a bit too quick.

sage: E = EllipticCurve('11a1')
sage: phi = E.isogeny(None,codomain=EllipticCurve('11a2'),degree=5)
sage: [phi(P) for P in E.torsion_points()]
[(0 : 1 : 0), (1/3 : 1/2 : 1), (1/3 : 1/2 : 1), (1/3 : 1/2 : 1), (1/3 : 1/2 : 1)]

again the images are not even on the codomain(). I.e. there is probably a second spot that needs a small change.

Changed 11 years ago by wuthrich

comment:3 Changed 11 years ago by wuthrich

  • Status changed from needs_work to needs_review

I think that should do it also for the kohel part.

comment:4 Changed 11 years ago by cremona

What about lines 981, 1002, in the patched file? They both say

          return self.__E2(0)

so shouldn't they also be changed to return 0 on the correct codomain?

comment:5 follow-up: Changed 11 years ago by wuthrich

No, these two lines must stay as they are. They do the right thing.

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

  • Authors set to Chris Wuthrich
  • Reviewers set to John Cremona
  • Status changed from needs_review to positive_review

Replying to wuthrich:

No, these two lines must stay as they are. They do the right thing.

OK, I trust you -- I tried to find an example where they did not do the right thing, and could not.

I'm happy -- patch (just the 2nd one) applies to 4.3.3 and test pass.

comment:7 Changed 11 years ago by wuthrich

  • Authors changed from Chris Wuthrich to Craig Citro, Chris Wuthrich

comment:8 Changed 11 years ago by mvngu

  • Merged in set to sage-4.3.4.alpha0
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.