#5976: [with new patch; with review] Add an Elliptic Curve Isogeny object
---------------------------+------------------------------------------------
Reporter: shumow | Owner: shumow
Type: enhancement | Status: assigned
Priority: major | Milestone: sage-4.0.1
Component: number theory | Keywords: Elliptic Curves
---------------------------+------------------------------------------------
Comment(by cremona):
I don't have time at the moment to go into this in a lot of detail. The
patch applies fine; the examples I tried (including a 37-isogeny over Q)
worked perfectly.
I have a few minor comments listed below. I found a lot of minor glitches
in the docstrings when building the reference manual, and have fixed
these. My patch was supposed to be applied after yours, but owing to a
mistake on my part is is a self-contained patch including all your code.
Sorry about that, I know that it makes it harder to see what I changed --
docstrings only!
It might be a good idea if David K also looked at it, since he is more
familiar with "his" algorithm than I am; I also know nothing about
Stark's algorithm.
Overall this is a very impressive piece of work (over 4000 lines!) and I
think it should go in as soon as possible, since it is only when people
start to use it for real that problems (if there are any) will surface.
Now for the (mainly) minor points:
* Several ReST glitches (e.g. missing double ::). [fixed]
* In the doctests for the helper functions I think it would be better
to
have direct rather than indirect doctests. For example in the tests
at
lines 98-99 surely the fact that the twosides agree is a tautology
since the same code is used on both sides? And in
compute_codomain_formula(), why not put in a direct doctest. (In
that
function you do not really need A1,A2,A3 and you can define
a1,...,a6
in one line via a1,a2,a3,a4,a6=E.ainvs(). Similarly elsewhere.)
* compute_vw_kohel_even_deg1() does not use parameter a6.
* l.225: incomplete sentence! [look for "missing text" in the source,
I put in at least 2]
* I like the very clear list of private data for the isogeny class,
where all are set to None by default, though I suspect this is more
like C++ than Python. It makes it *very* much easier to see what is
going on.
* There are lots of good examples.
* The docstring for the isogeny method for elliptic curves should say
more: it should certainly detail the input parameters and give more
examples (possibly copied from the EllipticCurveIsogeny
documentation). The point is the when someone E.<tab> and sees
E.isogeny is available and then types E.isogeny? they need to see
how
to use that function.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/5976#comment:31>
Sage <http://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
-~----------~----~----~----~------~----~------~--~---