#11429: Count integral points without PALP
----------------------------------+-----------------------------------------
Reporter: vbraun | Owner: AlexGhitza
Type: enhancement | Status: needs_review
Priority: major | Milestone: sage-4.7.1
Component: geometry | Keywords: sd31
Work_issues: | Upstream: N/A
Reviewer: Andrey Novoseltsev | Author: Volker Braun
Merged: | Dependencies: #11312
----------------------------------+-----------------------------------------
Comment(by novoselt):
Line numbers are for the new module in the big patch:
1. 224: should be `if A is not None:` for robustness (zero matrices
evaluate to `False`)
1. 231: should be `if A is None:`
1. 273-275: Why there is a conversion to set? Is it expected that the
output contains repetitions?
1. 293: shouldn't `translate_points` be used here?
1. 336: No description of input/output and what happens if the polyhedron
is not specified.
1. In the code of this function: why is it necessary to use this
permutation? It seems like an extra operation on all of the points,
although I believe that there is some reason ;-) Perhaps it can be
explained in the documentation or comments.
1. 421: Incomplete INPUT, missing OUTPUT, no real EXAMPLE.
1. 440: While this function is inaccessible from Sage directly, it still
would be nice to have a comment on what it does and what is d.
1. 477: If I am correct, this condition is always true. In which case it
can be removed, as well the next line, and 444 moved into the beginning of
outer while. (Just for making code a little simpler.)
1. I didn't yet looked carefully at the rest, but just one more pick at
575: Is it really necessary to have `DEF INEQ_INT_MAX_DIM = 20`? I mean
can't it be without hard-coded limits?
Will look over the second half of the new module in the next few days, I
am fine with the rest of the code.
Speedwise PALP is still more efficient for small polytopes, but of course
only if one can call PALP at once for a lot of them, which is not always
possible:
{{{
sage: len(ReflexivePolytopes(3))
4319
sage: %time
sage: ps = [Polyhedron(vertices=p.vertices().columns()) for p in
ReflexivePolytopes(3)]
CPU time: 83.87 s, Wall time: 218.85 s
sage: %time
sage: for p, lp in zip(ps, ReflexivePolytopes(3)):
... if len(p.integral_points()) != lp.npoints():
... print lp
CPU time: 10.10 s, Wall time: 10.30 s
sage: %time
sage: lattice_polytope.all_points(ReflexivePolytopes(3))
CPU time: 2.90 s, Wall time: 3.46 s
}}}
The last line is actually somewhat sad: PALP spends half a second to
compute all the points and then Sage spends 3 seconds to read its output
and convert into matrices. But hopefully objects of new generations will
be more efficient ;-)
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/11429#comment:11>
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.
