#13951: (non)archimedian_local_height broken for rational points on elliptic
curves
over Q
-----------------------------------+----------------------------------------
Reporter: pbruin | Owner: cremona
Type: defect | Status: needs_review
Priority: major | Milestone: sage-5.10
Component: elliptic curves | Resolution:
Keywords: local heights | Work issues:
Report Upstream: N/A | Reviewers:
Authors: Peter Bruin | Merged in:
Dependencies: #12509, #13953 | Stopgaps:
-----------------------------------+----------------------------------------
Comment (by cremona):
A second comment, of a different kind: how can it have happened and never
been noticied that these related functions are all spelled wrong! It is
"archimedean" and not "archimedian" (think "Archimedes").
I don't think I will be able to resist correcting all these: about 24
occurrences including doctests. Interestingly, in almost all places the
word is spelled correctly in comments. But should we just deprecate the
old spelling? I will consult sage-devel.
1. In line 2858 of the patched file I removed the division by
K.degree() since that is 1. This appeared to cut the time to doctest this
file (with --long) by a few seconds, but that might have been a freak!
2. The global height function would now work without calling pari,
hence with no special case needed for K==QQ. Alternatively, we could
easily add an "algorithm" parameter which could be either "sage" or
"pari", with "pari" the default. I think that over QQ pari uses Mestre's
AGM method for the real place, which is likely to be faster. Having the
choice of algorithm would make it easy to add a doctest to compare the
two; there should already be a doctest showing that the global height is
the sum of the arch. and non-arch. heights which would actually be a test
since we are computing them entirely indepependently!
3. The non-arch. height has a parameter "weighted" which is not
documented and should be, with doctests shoing its use. Similarly with
the is_minimal parameter. And for consistency, surely the arch. height
should also have a weighted parameter?
Sorry to be awkward; mathematically the patch is certainly ok!
I have a second patch which deals only with (1) spelling corrrections, but
no deprecation warning, (2) not dividing by K.degree() when it is 1. I
can add to that after this discussion, so will not upload it yet.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/13951#comment:9>
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 unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/sage-trac?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.
