#7324: [with patch, needs review] improve order_from_multiple
---------------------------+------------------------------------------------
Reporter: ylchapuy | Owner: tbd
Type: enhancement | Status: needs_review
Priority: major | Milestone: sage-4.2.1
Component: algebra | Keywords:
Work_issues: | Author: Yann Laigle-Chapuy
Reviewer: | Merged:
---------------------------+------------------------------------------------
Comment(by fwclarke):
This is a significant improvement, and it does apply to 4.2. I'd give a
fully positive review, but I've noticed a couple of things about this
function which could be considered.
Most important, the function always checks whether `M*P` equals the
identity. When this function is used one will normally be sure of the
order of the group (or of a subgroup in which the element lies), so that
this verification is unnecessary. I think the function should have an
optional parameter `check` (with default value `True` for backwards
compatibility) and that the `assert` line should be executed only if
`check is True`.
I found that your GF(3^108^) example ran about 25% faster with the
`assert` line commented out.
I noticed that `plist` now only gets used to create the factorization `F`,
and to check whether `M` is prime.
Thus the line
{{{
plist = [p for p,e in F]
}}}
isn't really needed. This leads me to
think that really the factorization of `M` is what should be cached
by the caller, for giving `plist` requires that the exponents get computed
each time the
function is called. Thus maybe there should be an optional parameter
`factorization` (with `plist` kept for compatibility), with code such as
such as
{{{
if factorization:
F = factorization
elif plist:
F = [(p, M.valuation(p)) for p in plist]
else:
F = M.factor()
if list(F) == [(M, 1)]:
return M
}}}
I notice that your GF(3^108^) example is nearly 4 times faster than
`a.multiplicative_order()`, and I've opened ticket #7324 for this function
to use `order_from_multiple`.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/7324#comment:4>
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
-~----------~----~----~----~------~----~------~--~---
