#3416: Weierstrass form for cubics
-------------------------------+--------------------------------------------
Reporter: moretti | Owner: was
Type: enhancement | Status: needs_work
Priority: major | Milestone: sage-4.7.1
Component: elliptic curves | Keywords: nagell, weierstrass, cubic,
elliptic curves, editor_wstein
Work_issues: | Upstream: N/A
Reviewer: Marco Streng | Author: Niels Duif
Merged: | Dependencies:
-------------------------------+--------------------------------------------
Comment(by mstreng):
Ok, here are some more comments, and a few cases where your code does not
work:
Most importantly: I don't like the output strings, I would definitely
prefer actual {{{SchemeMorphism_on_points_projective_space}}} objects.
This makes the method much more useful (as you can just evaluate these in
points to transfer points between the curves). It also makes your patch
much easier to review (as reviewers don't need to manually copy printed
strings to test correctness of your output). I don't think this change is
a lot of work: you can just feed your polynomials to E.hom, and you can
find an example in the parametrization function
[http://trac.sagemath.org/sage_trac/attachment/ticket/727/trac_727_conics_without_number_fields.patch
here]. I'm afraid it would require accepting an actual plane projective
curve as input `f` instead of a polynomial. For backwards compatibility,
you can change polynomials into curves when you get them as input.
Anyway, here are some comments on the code as it is:
If you stick with printing: in the description of the ``equivalence``
parameter: replace "given" by "*printed*" to make clear that it is only
printed, not really given as a Sage object. The "*"'s around "printed"
make it italic for emphasis.
I would have called the "equivalence" parameter "transformation" instead.
In all your examples:
{{{
sage: var("x y z")
(x, y, z)
sage: R.<x,y,z>=QQ[]
sage: f=R(x^3+y^3+z^3)
}}}
The second input overwrites the x, y, and z of the first input, so the
first two lines (one input and one output) can (and hence should) be
removed. Also, {{{x^3+y^3+z^3}}} is already in R, so remove "R(" and ")".
I don't understand the purpose of the "Then multiply the equation with".
The three substitutions already give a map in projective space, right?
Your function seems to require the variable names to be "x,y,z", this is
not right:
{{{
sage: Q.<a,b,c>=QQ[]
sage: EllipticCurve_from_cubic(a^3+b^3+60*c^3, [1,-1,0], equivalence=True)
…
KeyError: 'y'
}}}
There are cases that your code cannot handle:
{{{
sage: P.<x,y,z> = QQ[]
sage: EllipticCurve_from_cubic(x^3+y^3+z^3, [-1,1,0])
Elliptic Curve defined by y^2 + 2*x*y + 1/3*y = x^3 - x^2 - 1/3*x - 1/27
over Rational Field
sage: EllipticCurve_from_cubic(x^3+y^3+z^3, [-1,0,1])
…
ZeroDivisionError:
}}}
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/3416#comment:27>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/sage-trac?hl=en.
