Opened 14 months ago
Closed 12 months ago
#29190 closed enhancement (fixed)
Dual for combinatorial polyhedron
Reported by: | gh-kliem | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-9.1 |
Component: | geometry | Keywords: | polar, dual, combinatorial polyhedron |
Cc: | jipilab, gh-LaisRast | Merged in: | |
Authors: | Jonathan Kliem | Reviewers: | Laith Rastanawi |
Report Upstream: | N/A | Work issues: | |
Branch: | 73cf39f (Commits, GitHub, GitLab) | Commit: | 73cf39f5228f42fec3ad2a401ac9e6f535887d7d |
Dependencies: | Stopgaps: |
Description (last modified by )
We implement a method of CombinatorialPolyhedron
that obtains the polar/dual. We name it dual
and create an alias polar
to be consistent with Polyhedron_base
.
This is obtained by simply copying and interchanging the bitrepresentation of facets and vertices. The object is purely combinatorial and does not have vertex or facet names. A ValueError
is raised for the unbounded case.
Along the way we implement a __copy__
method for ListOfFaces
and allow initializing CombinatorialPolyhedron
from a tuple consisting of two ListOfFaces
.
Change History (10)
comment:1 Changed 14 months ago by
- Branch set to public/29190
- Commit set to 5078b6e7cd68c0eb9a3570fbef1beb9cab241297
- Status changed from new to needs_review
comment:2 Changed 14 months ago by
- Status changed from needs_review to needs_work
- Maybe add #28608 as a dependency here and change the code accordingly.
- In the documentation of
CombinatorialPolyhedron
. add tofar_face
the option whendata
is a tuple consisting of facets and vertices (of class ListOfFaces?). - A typo:
- * or a tuple consting of facets and vertices as two + * or a tuple consisting of facets and vertices as two
comment:3 Changed 14 months ago by
- Dependencies set to #28608
comment:4 Changed 14 months ago by
I'm just going to wait until #28608 is merged, otherwise it's a pain to review.
comment:5 Changed 14 months ago by
- Branch changed from public/29190 to public/29190-reb
- Commit changed from 5078b6e7cd68c0eb9a3570fbef1beb9cab241297 to 8beef7ca7c82e837629544b404eb2e47699caf56
- Dependencies #28608 deleted
- Status changed from needs_work to needs_review
comment:6 Changed 13 months ago by
- Status changed from needs_review to needs_work
- I would suggest renaming the method name to
dual
(since this is purely combinatorial) and putpolar
as an alias todual
to be consistent withPolyhedron_base
.
- Add
.. SEEALSO:: :meth:`~sage.geometry.polyhedron.base.Polyhedron_base.polar`.
Otherwise, the ticket is good to go.
comment:7 Changed 13 months ago by
- Commit changed from 8beef7ca7c82e837629544b404eb2e47699caf56 to 73cf39f5228f42fec3ad2a401ac9e6f535887d7d
comment:8 Changed 13 months ago by
- Description modified (diff)
- Reviewers set to Laith Rastanawi
- Status changed from needs_work to needs_review
- Summary changed from Polar for combinatorial polyhedron to Dual for combinatorial polyhedron
comment:9 Changed 13 months ago by
- Status changed from needs_review to positive_review
I think it is good to go.
comment:10 Changed 12 months ago by
- Branch changed from public/29190-reb to 73cf39f5228f42fec3ad2a401ac9e6f535887d7d
- Resolution set to fixed
- Status changed from positive_review to closed
New commits:
copy for ListOfFaces
add polar for combinatorial polyhedron