#8799: Bring doctests for mwrank.pyx up to 100% (from 3%)
-----------------------------------------------+----------------------------
Reporter: cremona | Owner: cremona
Type: defect | Status: needs_review
Priority: major | Milestone: sage-5.0
Component: documentation | Keywords: mwrank
Author: John Cremona | Upstream: N/A
Reviewer: Minh Van Nguyen, Leif Leonhardy | Merged:
Work_issues: |
-----------------------------------------------+----------------------------
Comment(by leif):
Replying to [comment:19 cremona]:
> I did not see the typo in the code which Leif mentioned. Is it where we
test height_bound > 21.4 and not 21.48?
No. I meant:
{{{
- verbose == bool(verbose)
+ verbose = bool(verbose)
}}}
> In that place, it is true that this could be approximately doubled on
64-bit machines (I guess that when this code was first written the person
doing it did not know how to test that) but in practice using bounds > 21
will take a very very long time.
It's just that the implementation doesn't meet the documentation:
{{{
- ``height_limit`` (float, default: 18) -- search up to this
logarithmic height.
.. note::
On 32-bit machines, this *must* be < 21.48 else
`\exp(h_{\text{lim}}) > 2^{31}` and overflows. On 64-bit
machines, it
must be *at most* 43.668. However, this bound is a logarithmic
bound and increasing it by just 1 increases the running time
by (roughly) `\exp(1.5)=4.5`, so searching up to even 20
takes a very long time.
}}}
(We could add here that larger heights for 64-bit code currently aren't
implemented in the Python interface.)
{{{
height_limit = float(height_limit)
if height_limit >= 21.4: # TODO: docstring says 21.48 (for
32-bit machines; what about 64-bit...?)
raise ValueError, "The height limit must be < 21.4."
}}}
Also, if the docstring says {{{21.48}}}, the code should use the same
value.
(Note that the code should check for what machine the code/{{{mwrank}}}
was compiled for, not what machine Sage is currently running on...)
But that (change of the implementation, and perhaps other TODOs, like in
{{{mwrank_EllipticCurve.__repr__()}}}) should be fixed on another ticket.
[[BR]]
> [...] some of the points raised on this ticket deserve a wider audience,
possibly additional sentences in the developers guide?
Definitely. :)
-Leif
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/8799#comment:20>
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.
