#28850 closed defect (fixed)

Polar of polytopes does not check if polytope is full-dimensional

Reported by: gh-kliem Owned by:
Priority: major Milestone: sage-9.0
Component: geometry Keywords: polar, polytopes
Cc: jipilab, gh-LaisRast Merged in:
Authors: Jonathan Kliem Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 4ee9802 (Commits, GitHub, GitLab) Commit: 4ee9802afe3ef9e893e95756c980163146dd83fa
Dependencies: Stopgaps:

Status badges

Description (last modified by gh-kliem)

Currently, the polar of rational polyhedra does not check, whether the polyhedron is full-dimensional:

sage: P = polytopes.simplex(3, base_ring=QQ)
sage: P.polar()
A 4-dimensional 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 full-dimensional

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 3-dimensional 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 gh-kliem

  • Description modified (diff)

comment:2 Changed 16 months ago by gh-kliem

  • Branch set to public/28850
  • Commit set to 6b12213231dd658db0fe44791e04d250a054699d
  • Status changed from new to needs_review

I'm not exactly happy with the phrasing and the name in_affine_span.


New commits:

6b12213check whether polytope is full-dimensional before taking the polar; added a polar version in its affine span

comment:3 Changed 16 months ago by gh-kliem

phrasing in the doctests

comment:4 Changed 16 months ago by gh-kliem

  • Keywords polar polytopes added

comment:5 follow-up: Changed 16 months ago by 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 a ValueError or TypeError. Otherwise LGTM.

comment:6 Changed 16 months ago by git

  • Commit changed from 6b12213231dd658db0fe44791e04d250a054699d to 4ee9802afe3ef9e893e95756c980163146dd83fa

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

4ee9802alignment in docs; AssertionError -> ValueError

comment:7 in reply to: ↑ 5 Changed 16 months ago by gh-kliem

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 a ValueError or TypeError. Otherwise LGTM.

comment:8 Changed 16 months ago by tscrim

  • Reviewers set to Travis Scrimshaw
  • Status changed from needs_review to positive_review

Thanks.

comment:9 Changed 16 months ago by vbraun

  • Branch changed from public/28850 to 4ee9802afe3ef9e893e95756c980163146dd83fa
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.