Opened 16 months ago
Closed 16 months ago
#28850 closed defect (fixed)
Polar of polytopes does not check if polytope is fulldimensional
Reported by:  ghkliem  Owned by:  

Priority:  major  Milestone:  sage9.0 
Component:  geometry  Keywords:  polar, polytopes 
Cc:  jipilab, ghLaisRast  Merged in:  
Authors:  Jonathan Kliem  Reviewers:  Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  4ee9802 (Commits, GitHub, GitLab)  Commit:  4ee9802afe3ef9e893e95756c980163146dd83fa 
Dependencies:  Stopgaps: 
Description (last modified by )
Currently, the polar of rational polyhedra does not check, whether the polyhedron is fulldimensional:
sage: P = polytopes.simplex(3, base_ring=QQ) sage: P.polar() A 4dimensional polyhedron in QQ^4 defined as the convex hull of 4 vertices and 1 line
We fix this by adding an assertion
sage: P = polytopes.simplex(3, base_ring=QQ) sage: P.polar() Traceback (most recent call last): ... AssertionError: must be fulldimensional
Also we add an extra keyword in_affine_span
(default False
). By this one can obtain the polar in its affine span (after translation as usual):
sage: P = polytopes.simplex(3, base_ring=QQ) sage: P.polar(in_affine_span=True) A 3dimensional polyhedron in QQ^4 defined as the convex hull of 4 vertices sage: point = Polyhedron([[0]]) sage: P = polytopes.cube().change_ring(QQ) sage: (P*point).polar(in_affine_span=True) == P.polar()*point True
This option seems reasonable and simplifies the current construction of barycentric subdivision.
We change the other message "Not a polytope."
according to conventions to "not a polytope"
.
Change History (9)
comment:1 Changed 16 months ago by
 Description modified (diff)
comment:2 Changed 16 months ago by
 Branch set to public/28850
 Commit set to 6b12213231dd658db0fe44791e04d250a054699d
 Status changed from new to needs_review
comment:3 Changed 16 months ago by
phrasing in the doctests
comment:4 Changed 16 months ago by
 Keywords polar polytopes added
comment:5 followup: ↓ 7 Changed 16 months ago by
Check that :trac:`28850` is fixed::
is overindented. Also it is not considered good practice to do an assert
to check input but instead raise a ValueError
or TypeError
. Otherwise LGTM.
comment:6 Changed 16 months ago by
 Commit changed from 6b12213231dd658db0fe44791e04d250a054699d to 4ee9802afe3ef9e893e95756c980163146dd83fa
Branch pushed to git repo; I updated commit sha1. New commits:
4ee9802  alignment in docs; AssertionError > ValueError

comment:7 in reply to: ↑ 5 Changed 16 months ago by
Ok, done. I also changed the test for compactness to give a ValueError
.
Replying to tscrim:
Check that :trac:`28850` is fixed::is overindented. Also it is not considered good practice to do an
assert
to check input but instead raise aValueError
orTypeError
. Otherwise LGTM.
comment:8 Changed 16 months ago by
 Reviewers set to Travis Scrimshaw
 Status changed from needs_review to positive_review
Thanks.
comment:9 Changed 16 months ago by
 Branch changed from public/28850 to 4ee9802afe3ef9e893e95756c980163146dd83fa
 Resolution set to fixed
 Status changed from positive_review to closed
I'm not exactly happy with the phrasing and the name
in_affine_span
.New commits:
check whether polytope is fulldimensional before taking the polar; added a polar version in its affine span