Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Polyhedron.is_simple() #27533

Closed
kliem opened this issue Mar 22, 2019 · 15 comments
Closed

Improve Polyhedron.is_simple() #27533

kliem opened this issue Mar 22, 2019 · 15 comments

Comments

@kliem
Copy link
Contributor

kliem commented Mar 22, 2019

The method Polyhedron.is_simple is pretty slow for large polytopes at the moment. There is no need for that, as the information can be directly retrieved from the Vrepresentation.

Current timings:

sage: P = polytopes.hypercube(6)
sage: %time P.is_simple()
CPU times: user 360 ms, sys: 8 ms, total: 368 ms
Wall time: 364 ms
True
sage: P = polytopes.hypercube(7)
sage: %time P.is_simple()
CPU times: user 1.78 s, sys: 48 ms, total: 1.83 s
Wall time: 1.74 s
True
sage: P = polytopes.cross_polytope(7)
sage: %time P.is_simple()
CPU times: user 996 ms, sys: 0 ns, total: 996 ms
Wall time: 992 ms
False

Timings with this ticket:

sage: P = polytopes.hypercube(6)
sage: %time P.is_simple()
CPU times: user 4 ms, sys: 4 ms, total: 8 ms
Wall time: 3.53 ms
True
sage: P = polytopes.hypercube(7)
sage: %time P.is_simple()
CPU times: user 4 ms, sys: 4 ms, total: 8 ms
Wall time: 5.39 ms
True
sage: P = polytopes.cross_polytope(7)
sage: %time P.is_simple()
CPU times: user 8 ms, sys: 0 ns, total: 8 ms
Wall time: 5.63 ms
False
sage: P = polytopes.permutahedron(4)
sage: %time P.is_simple()
CPU times: user 0 ns, sys: 0 ns, total: 0 ns
Wall time: 1.03 ms
True

Component: geometry

Author: Jonathan Kliem

Branch/Commit: cd714a1

Reviewer: Laith Rastanawi

Issue created by migration from https://trac.sagemath.org/ticket/27533

@kliem kliem added this to the sage-8.8 milestone Mar 22, 2019
@kliem
Copy link
Contributor Author

kliem commented Mar 22, 2019

Last 10 new commits:

3ec32e8Merge branch 'public/upgrade_polymake_to_version_3_2r2' of trac.sagemath.org:sage into new_24905
f4dee94Updated polymake to 3.2r4
d5b94d6Fixed quotes and wiggled around
b2e133cFixed the typeof
d9af561small fixes
40c6098pep8
a5bf869Fixed backend polymake
7a5de5aMerge branch 'public/upgrade_polymake_to_version_3_2r4' of trac.sagemath.org:sage into polymake32r4
606f53fMerge branch 'public/upgrade_polymake_to_version_3_2r4' of git://trac.sagemath.org/sage into develop
bd10eb3Improved method ``is_simple``

@kliem
Copy link
Contributor Author

kliem commented Mar 22, 2019

Branch: public/27533

@kliem
Copy link
Contributor Author

kliem commented Mar 22, 2019

Commit: bd10eb3

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 22, 2019

Changed commit from bd10eb3 to f6aa8c6

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 22, 2019

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

392ca56Merge branch 'develop' of git://trac.sagemath.org/sage into develop
f6aa8c6improved ``Polyhedron.is_simple()``

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 22, 2019

Changed commit from f6aa8c6 to ec41ee1

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 22, 2019

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

ec41ee1improve is_simple

@kliem
Copy link
Contributor Author

kliem commented Mar 22, 2019

Changed branch from public/27533 to public/27533_new

@kliem
Copy link
Contributor Author

kliem commented Mar 22, 2019

Changed commit from ec41ee1 to cd714a1

@kliem
Copy link
Contributor Author

kliem commented Mar 22, 2019

comment:6

Had to fix my git develop branch


New commits:

cd714a1improved is_simple

@kliem
Copy link
Contributor Author

kliem commented Mar 22, 2019

Changed branch from public/27533_new to public/27533

@kliem
Copy link
Contributor Author

kliem commented Mar 22, 2019

Author: Jonathan Kliem

@LaisRast
Copy link

LaisRast commented Apr 5, 2019

comment:9

This is much faster than the previous way.
It looks fine to me.

@LaisRast
Copy link

LaisRast commented Apr 5, 2019

Reviewer: Laith Rastanawi

@vbraun
Copy link
Member

vbraun commented Apr 7, 2019

Changed branch from public/27533 to cd714a1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants