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
Weierstrass form and Jacobian for cubics and certain other genus-one curves #3416
Comments
comment:1
Attachment: weier.patch.gz Since y^2 = quartic is also genus 1, I'm curious if there is a corresponding function planned to compute the Weierstrass form of a quartic hyperelliptic. |
comment:2
You can't give your own code a negative review :-). I changed it to "not ready for review". |
Changed keywords from nagell, weierstrass, cubic to nagell, weierstrass, cubic, editor_wstein |
Attachment: Weierstrass.sage.gz Algorithm 7.4.10 of GTM 138 |
comment:5
I believe the formulas used here are incorrect. I'm not sure, but they appear similar to the erroneous formula's from Cohen's book. I'm currently working on an implementation that uses different formulas. I expect that to be done within two weeks. One of my colleagues is working on y2 = quartic, which I will implement for him. I expect this to be done within a couple of weeks. During Sage Days 23 plans were made for other curves too, but I'm not sure anyone is working on those. |
comment:6
Replying to @sagetrac-Niels:
Yes, they are definitely the wrong formulas in Cohen's book. -- William |
comment:7
Henri Cohen writes (2010-08-12):
This suggests that very minor corrections are all that is required. |
comment:8
I have found that the descriptions in: Cassels, J. W. S. Lectures on elliptic curves. London Mathematical Society Student Texts, 24. Cambridge University Press, Cambridge, 1991. give excellent recipes for writing down isomorphisms from cubics, y!^2=quartic and intersections of two quadrics in P!^2 with a rational point to their Jacobians in weierstrass form. The alternative method of writing down maps of degrees 3, 2, 4 (respectively) to their Jacobians using invariant theory (see Tom Fisher et al.) often results in nicer models, because there are less arbitrary choices to be made. |
comment:9
Replying to @nbruin:
Sure, and it would be great to have all these genus one modes in Sage! (you can call me) al |
comment:10
Replying to @nbruin:
Thanks for the advice. I will browse through the suggested literature to find the most convenient way of implementing cubic to Weierstrass. I plan to finish this before the end of August. If I have more time I will try some quartics and see whether I can get the nicer formulas using J-invariants. |
Attachment: 14532.patch.gz |
comment:11
Attachment: cnotes.pdf.gz Here is my first ever Sage patch: 14532.patch. It creates an elliptic curve from any homogeneous cubic. The cubic is transformed to Weierstrass, and birational maps between the equations are given. The case y2 = quartic is not covered. The patch is based upon Sage 4.4.4. The used transformations are from Section 4.4 of the course notes: "G1CRPC: Rational Points on Curves", which I uploaded as cnotes.pdf. Please review my patch, and provide extensive feedback. |
comment:13
Just a couple of quick comments, as I have not yet had a chance to look at the patch properly or try it out. First: I'm not sure we really want my old lecture notes attached to this ticket! There is nothing you are using there that is not in standard other sources. But if they are there, they should be attributed ;) Secondly, you absolutely cannot have print statements giving part of the output as a side-effect. Better to return a tuple consisting of the elliptic curve and the morphisms. This will break backwards compatibility, since the Magma version does not so this (though he underlying Magma function surely does), but it could be controlled by an extra parameter which defaults to False. |
comment:14
Replying to @JohnCremona:
The problem is that most of the standard other sources contain incorrect formulas. These lecture notes are the first source I found with correct formulas. The main reason for including them is that it may help people in reviewing the patch. If anyone finds the same transformations in a standard source, please let me know.
Ok, I can change that. Is there nice a way to represent such a morphism in Sage? I have difficulty expressing a transformation like the following as a single morphism (just an example from the top of my head): x -> x2 - y y -> x2 + y z -> z2 Then multiply with 6/(x2 z) |
comment:15
I tried to define the morphism properly, which should work, but it doesn't since in defining the morphism Sage checks that the polynomials used a homogeneous of the same degree (see line 458 of schemes.generic.morphism.py), but the degree function is not defined for elements of the coordinate ring. This can be fixed: although in general QuotientRingElement s have no degree, in this situation (where the quotient is by a homogeneous ideal) the degree is well-defined. |
comment:16
See #10297 for a separate report on this (and, soon, a patch). |
comment:17
Replying to @JohnCremona:
The patch is there, so please review it! The example I used there is one of the examples from the patch here. Replacing the curve E used there with
successfully defines the morphism. I recommend that the function here just returns the morphism, since one can recover E from
This will not be the end of the story, as I now cannot apply f to a point on C to get a point on E, but that's because of another difficulty like the one at #10297, so should be fixed separately. |
comment:78
I'm now happy to give this a positive review -- at last! Thanks to Volker for the hard work which gives a lot more than this ticket originally promised. I do not feel qualified to positively review #13458 though, so I hope this is not being inconsistent of me. |
comment:80
Sorry, there are some issues with these patches: Most importantly, the doctests for Then some details: I would replace
by
or
And also replace
by
|
comment:81
I didn't write the The attached patch changes the function call so that it is doctested. |
This comment has been minimized.
This comment has been minimized.
comment:83
Can any of the "official" reviewers of this ticket please check that these doctests pass with magma with
|
comment:84
Replying to @jdemeyer:
I just did, and there are some problems, which I am now fixing. I note that the dependent #13458 is marged as having been merged, but only in 5.12.beta0 which is in the future for most of us! John |
comment:85
Am I doing something stupid here? I have 5.11.rc0 with the patches from #13458 and this ticket applied. Now
despite
The relevant line works fine by itself:
|
comment:86
I've added the missing import, should work now. |
comment:87
Replying to @vbraun:
That's odd as I would not have expected it to be necessary to import SR for a doctest. Anyway, that function is still wrong since 2 lines later it refers to P which I presume is a point on the curve, but there is no parameter P. Shall we just delete this completely redundant function? The next function
|
Attachment: trac_3416_magma.patch.gz Updated patch |
comment:88
Updated patch removes both |
comment:89
Replying to @vbraun:
Now the optional tests certainly pass! Back to positive review, and I hope that Jeroen is happy. |
comment:90
Replying to @JohnCremona:
Obviously :-)
Sure! |
Merged: sage-5.12.beta3 |
comment:92
See #15340 for a follow-up. |
Implement transformations to put a general cubic (with a point) into Weierstrass form:
Also, Jacobians (without specifying point):
Apply:
See attachment: cubic_to_weierstrass_documentation.pdf for details on how the algorithm works.
Depends on #12553
Depends on #13084
Depends on #13458
CC: @novoselt
Component: elliptic curves
Keywords: nagell, weierstrass, cubic, editor_wstein
Author: Niels Duif, Volker Braun
Reviewer: John Cremona, Marco Streng, Nils Bruin
Merged: sage-5.12.beta3
Issue created by migration from https://trac.sagemath.org/ticket/3416
The text was updated successfully, but these errors were encountered: