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: |
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)
Change History (10)
Changed 11 years ago by
comment:1 Changed 11 years ago by
- Status changed from new to needs_review
comment:2 Changed 11 years ago by
- 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
comment:3 Changed 11 years ago by
- 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
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: ↓ 6 Changed 11 years ago by
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
- 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
comment:8 Changed 11 years ago by
- Merged in set to sage-4.3.4.alpha0
- Resolution set to fixed
- Status changed from positive_review to closed
Merged trac_8349.2.patch.
Attached a quick fix -- I'm happy to let it be ignored if there's something classier to be done.