Closed kliem closed 4 years ago
Branch: public/28873
Branch pushed to git repo; I updated commit sha1. New commits:
a35dc82 | removed not needed method |
Changed branch from public/28873 to public/28873-reb
Branch pushed to git repo; I updated commit sha1. New commits:
add61a5 | undo false alignment |
It should be noted that the conversion of the normaliz output does not work correctly when computing Sublattice
for algebraic polyhedra.
Description changed:
---
+++
@@ -44,6 +44,29 @@
64
-Follow up: +This also speeds up calculation in case the polyhedron is not full-dimensional:
-Once affine_hull
completely respects the backend, this method will be used to calculate the induced volume by default (automatically).
+ +sage: P = polytopes.permutahedron(6, backend='normaliz') +sage: %time P.volume(measure='induced') +CPU times: user 37.7 s, sys: 42.4 ms, total: 37.7 s +Wall time: 24.2 s +1296*sqrt(6) +sage: %time P.volume(engine='internal', measure='induced') +CPU times: user 42.9 s, sys: 71.9 ms, total: 43 s +Wall time: 41.8 s +1296*sqrt(6) +
+
+However, note that calculation of the affine hull takes quite some time in this case. So we leave the inexact normaliz option for now:
+
+ +sage: %time P.volume(engine='normaliz', measure='induced') +CPU times: user 710 ms, sys: 0 ns, total: 710 ms +Wall time: 92.3 ms +3174.5387066469984 +
+
+Possible follow ups:
+
+There is probably a good and fast way to convert the lattice volume to the euclidean volume exactly. Also, once we can set up a normaliz polyhedron with both Vrep and Hrep, affine hull should be really quick.
Ticket retargeted after milestone closed
Changed branch from public/28873-reb to public/28873-reb2
Batch modifying tickets that will likely not be ready for 9.1, based on a review of the ticket title, branch/review status, and last modification date.
Reviewer: Jean-Philippe Labbé
You should add the reference to the manual in the documentation of the function. That would be a good improvement.
Changed branch from public/28873-reb2 to public/28873-reb3
It would be nice to have this in 9.1, if possible.
Is the following "unimodual" a typo? Is unimodular meant?
+ One other possibility is to compute the scaled volume where a unimodual
There is also
+ volume of the unimodal simplex (or zero if not full-dimensional)::
Unimodular?
Could you check if this occurs in the rest of the file too?
Otherwise, the ticket looks good and the bots are green on 9.1rc0, so you can put this on positive review on my behalf once you considered the above comments.
Branch pushed to git repo; I updated commit sha1. New commits:
cc808dd | typo with unimodular |
Changed branch from public/28873-reb3 to cc808dd
We implement the ambient volume using backend normaliz.
This is done by return 0 or infinity if the polyhedron is not full-dimensional and not compact respectively.
Otherwise, we take the
induced_lattice
volume and divide byfactorial(self.dim())
.See section 6.1.1 of the normaliz manual on how
induced_lattice
volume relates to euclidean volume: https://github.com/Normaliz/Normaliz/blob/master/doc/Normaliz.pdfThis is much faster than the current method, so we set this to default if backend is normaliz.
This also speeds up calculation in case the polyhedron is not full-dimensional:
However, note that calculation of the affine hull takes quite some time in this case. So we leave the inexact normaliz option for now:
Possible follow ups:
There is probably a good and fast way to convert the lattice volume to the euclidean volume exactly. Also, once we can set up a normaliz polyhedron with both Vrep and Hrep, affine hull should be really quick.
Depends on #28872
CC: @jplab @LaisRast
Component: geometry
Keywords: polyhedron, normaliz, ambient volume
Author: Jonathan Kliem
Branch/Commit:
cc808dd
Reviewer: Jean-Philippe Labbé
Issue created by migration from https://trac.sagemath.org/ticket/28873