Opened 3 years ago

Closed 3 years ago

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

Reported by: Owned by: gh-kliem major sage-9.0 geometry polar, polytopes jipilab, gh-LaisRast Jonathan Kliem Travis Scrimshaw N/A 4ee9802 4ee9802afe3ef9e893e95756c980163146dd83fa

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"`.

### comment:1 Changed 3 years ago by gh-kliem

• Description modified (diff)

### comment:2 Changed 3 years 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:

 ​6b12213 `check whether polytope is full-dimensional before taking the polar; added a polar version in its affine span`

### comment:3 Changed 3 years ago by gh-kliem

phrasing in the doctests

### comment:4 Changed 3 years ago by gh-kliem

• Keywords polar polytopes added

### comment:5 follow-up: ↓ 7 Changed 3 years 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 3 years ago by git

• 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 3 years ago by gh-kliem

Ok, done. I also changed the test for compactness to give a `ValueError`.

```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 3 years ago by tscrim

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

Thanks.

### comment:9 Changed 3 years 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.