Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug in isogenies #8349

Closed
chriswuthrich opened this issue Feb 24, 2010 · 13 comments
Closed

bug in isogenies #8349

chriswuthrich opened this issue Feb 24, 2010 · 13 comments

Comments

@chriswuthrich
Copy link
Contributor

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

Component: elliptic curves

Keywords: isogeny

Author: Craig Citro, Chris Wuthrich

Reviewer: John Cremona

Merged: sage-4.3.4.alpha0

Issue created by migration from https://trac.sagemath.org/ticket/8349

@craigcitro
Copy link
Member

Attachment: trac_8349.patch.gz

@craigcitro
Copy link
Member

comment:1

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

@chriswuthrich
Copy link
Contributor Author

comment:2

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.

@chriswuthrich
Copy link
Contributor Author

Attachment: trac_8349.2.patch.gz

@chriswuthrich
Copy link
Contributor Author

comment:3

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

@JohnCremona
Copy link
Member

comment:4

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?

@chriswuthrich
Copy link
Contributor Author

comment:5

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

@JohnCremona
Copy link
Member

comment:6

Replying to @categorie:

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.

@JohnCremona
Copy link
Member

Reviewer: John Cremona

@JohnCremona
Copy link
Member

Author: Chris Wuthrich

@chriswuthrich
Copy link
Contributor Author

Changed author from Chris Wuthrich to Craig Citro, Chris Wuthrich

@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Mar 3, 2010

Merged: sage-4.3.4.alpha0

@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Mar 3, 2010

comment:8

Merged trac_8349.2.patch.

@sagetrac-mvngu sagetrac-mvngu mannequin removed the s: positive review label Mar 3, 2010
@sagetrac-mvngu sagetrac-mvngu mannequin closed this as completed Mar 3, 2010
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants