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

Implement division_field() for elliptic curves #11905

Closed
jdemeyer opened this issue Oct 7, 2011 · 45 comments
Closed

Implement division_field() for elliptic curves #11905

jdemeyer opened this issue Oct 7, 2011 · 45 comments

Comments

@jdemeyer
Copy link

jdemeyer commented Oct 7, 2011

sage/schemes/elliptic_curves/gal_reps.py has code to compute a splitting field, but that clearly does not belong to elliptic curves. This is used to compute the division field. We should use the new splitting_field() function from #2217 to implement this. We should make it work also over number fields.

Depends on #2217
Depends on #15626
Depends on #11271

CC: @JohnCremona

Component: elliptic curves

Author: Jeroen Demeyer

Branch/Commit: u/jdemeyer/ticket/11905 @ bb38871

Reviewer: John Cremona

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

@jdemeyer
Copy link
Author

jdemeyer commented Oct 7, 2011

Dependencies: #11891

@jdemeyer
Copy link
Author

jdemeyer commented Nov 7, 2011

Changed dependencies from #11891 to #11904, #11995, #2217

@jdemeyer

This comment has been minimized.

@jdemeyer jdemeyer assigned JohnCremona and unassigned loefflerd Nov 7, 2011
@jdemeyer jdemeyer changed the title Move _splitting_field from elliptic curves to rings Remove _splitting_field() from elliptic curves Nov 7, 2011
@chriswuthrich
Copy link
Contributor

comment:4

Thanks for spotting and changing this. One could even argue that the division field should be available to all elliptic curves, not just the ones over QQ. But this could be changed later.

@jdemeyer
Copy link
Author

jdemeyer commented Nov 8, 2011

comment:5

This is still very much work in progress by the way.

@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link
Author

comment:7

Replying to @categorie:

Thanks for spotting and changing this. One could even argue that the division field should be available to all elliptic curves, not just the ones over QQ. But this could be changed later.

I can easily do number fields, very general fields would be harder.

@jdemeyer
Copy link
Author

Attachment: 11905_splitting_field.patch.gz

@jdemeyer jdemeyer modified the milestones: sage-5.11, sage-5.12 Aug 13, 2013
@fchapoton
Copy link
Contributor

comment:9

#2217 is ready for review

@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link
Author

jdemeyer commented Jan 3, 2014

Author: Jeroen Demeyer

@jdemeyer
Copy link
Author

jdemeyer commented Jan 3, 2014

Branch: u/jdemeyer/ticket/11905

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 3, 2014

Commit: a76a510

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 3, 2014

Branch pushed to git repo; I updated commit sha1. This was a forced push. Recent commits:

a76a510 Implement division_field() for elliptic curves
c50eb3e No need to specify caller_name in verbose()
6df69b6 Add comments, small cosmetic changes
9e2dd44 Make q monic before computing cubic resolvent
9b5558e Implement splitting fields for number fields

@jdemeyer
Copy link
Author

jdemeyer commented Jan 3, 2014

Changed dependencies from #11904, #11995, #2217 to #2217

@jdemeyer
Copy link
Author

jdemeyer commented Jan 3, 2014

Changed dependencies from #2217 to #2217, #15626

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 3, 2014

Branch pushed to git repo; I updated commit sha1. This was a forced push. Recent commits:

d6a7f3c Implement division_field() for elliptic curves
776795d Do polynomial consistency check only for minimal dm
df52508 Further improvements to splitting_field()
c50eb3e No need to specify caller_name in verbose()
6df69b6 Add comments, small cosmetic changes

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 3, 2014

Changed commit from a76a510 to d6a7f3c

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 5, 2014

Changed commit from 810ceec to 3661e44

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 5, 2014

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

3661e44Implement division_field() for elliptic curves
ca46e2bDo polynomial consistency check only for minimal dm
7830eabFurther improvements to splitting_field()

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 5, 2014

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

9b406d0Implement division_field() for elliptic curves

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 5, 2014

Changed commit from 3661e44 to 9b406d0

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 5, 2014

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

4d450f1Implement division_field() for elliptic curves

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 5, 2014

Changed commit from 9b406d0 to 4d450f1

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 5, 2014

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

8b96090Implement division_field() for elliptic curves

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 5, 2014

Changed commit from 4d450f1 to 8b96090

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 5, 2014

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

05c57d2Implement division_field() for elliptic curves

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 5, 2014

Changed commit from 8b96090 to 05c57d2

@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link
Author

jdemeyer commented Jan 6, 2014

comment:28

Note that only 05c57d2 needs to be reviewed.

@jdemeyer

This comment has been minimized.

@jdemeyer jdemeyer changed the title Remove _splitting_field() from elliptic curves Implement division_field() from elliptic curves Jan 6, 2014
@JohnCremona
Copy link
Member

comment:30

Looking good! Can we have an example where the X-division field equals the division field, i.e. where the final step is not necessary? And also an example (from the ones you give) showing that the splitting field of the division polynomial has half the degree of the division field? I think that this would be instructive to see in the reference manual. I see that in the code for the final step you again call splitting field with a quadratic; very efficient!

I think it says a lot for the good way in which you implemented splitting fields that this now becomes so simple!

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 6, 2014

Changed commit from 05c57d2 to bb38871

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 6, 2014

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

bb38871division_field(): use map_coefficients() and add more examples

@JohnCremona
Copy link
Member

Reviewer: John Cremona

@JohnCremona
Copy link
Member

comment:32

I was expecting to be able to pull the new commit on top of the old one, but that failed (merge conflict I think). I just deleted the branch I had and made a new one.

Thanks for the new example, which do exactly what I wanted. I read through the code again, and like it, and tested all of Sage with this commit as well as some of my own examples.

@jdemeyer
Copy link
Author

jdemeyer commented Jan 6, 2014

comment:33

Thanks again for the quick review!

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

5 participants