Opened 4 years ago

Closed 4 years ago

#22309 closed enhancement (fixed)

Use PPL for computing vertices of LatticePolytope

Reported by: novoselt Owned by:
Priority: major Milestone: sage-7.6
Component: geometry Keywords:
Cc: vbraun Merged in:
Authors: Andrey Novoseltsev Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: c820307 (Commits, GitHub, GitLab) Commit: c82030713adafe2b3706df63fb65652c5fe846b2
Dependencies: #22304 Stopgaps:

Status badges

Description (last modified by novoselt)

This gives drastic improvement for small polytopes which is the target case. Before:

sage: timeit("LatticePolytope(lattice_polytope.cross_polytope(3).vertices())")
25 loops, best of 3: 35.2 ms per loop

After:

sage: timeit("LatticePolytope(lattice_polytope.cross_polytope(3).vertices())")
625 loops, best of 3: 684 µs per loop

Next in the chain of lattice polytope improvements is #22310

Change History (9)

comment:1 Changed 4 years ago by novoselt

  • Branch set to u/novoselt/PPL_for_vertices

comment:2 Changed 4 years ago by novoselt

  • Cc vbraun added
  • Commit set to db3b54e20e98572a34918526aa0e8c28c83dea78
  • Status changed from new to needs_review

Volker, I was going to stop bugging you, but this great speed up is mostly due to what you've done for cones, I've just adjusted it to work with points rather than rays.


New commits:

a262ec3Make dual nef-partitions conveniently ordered
6b5972cFix nef-partition ordering in doctests
2e26768Add NefCompleteIntersection.cohomology_class
7afca73Add Cayley polytopes/cones to dosctring of NefPartition
e78ff49Add PPL representation to LatticePolytope
5c286e4Use PPL for computing vertices of LatticePolytope
db3b54eFix doctests - mostly due to different order of vertices

comment:3 Changed 4 years ago by novoselt

  • Description modified (diff)

comment:4 Changed 4 years ago by tscrim

For multiline imports, you can use (IMO cleaner):

from sage.libs.ppl import (C_Polyhedron, Generator_System, Linear_Expression,
                           point as PPL_point)

Otherwise LGTM.

comment:5 Changed 4 years ago by tscrim

  • Reviewers set to Travis Scrimshaw

comment:6 Changed 4 years ago by git

  • Commit changed from db3b54e20e98572a34918526aa0e8c28c83dea78 to c82030713adafe2b3706df63fb65652c5fe846b2

Branch pushed to git repo; I updated commit sha1. New commits:

c820307Use parentheses for multiline import

comment:7 Changed 4 years ago by novoselt

Thank you! I've checked with PEP 8 that this is also the recommended way of doing things - will try to keep in mind for the future. So - positive review?-)

comment:8 Changed 4 years ago by tscrim

  • Status changed from needs_review to positive_review

Yes indeed.

comment:9 Changed 4 years ago by vbraun

  • Branch changed from u/novoselt/PPL_for_vertices to c82030713adafe2b3706df63fb65652c5fe846b2
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.