Opened 3 years ago
Closed 3 years ago
#28639 closed enhancement (fixed)
Polyhedron_normaliz: Initialize new cone from both Vrep and Hrep
Reported by: | gh-kliem | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-9.0 |
Component: | geometry | Keywords: | polyhedron, normaliz |
Cc: | jipilab, mkoeppe, Winfried | Merged in: | |
Authors: | Jonathan Kliem | Reviewers: | Travis Scrimshaw |
Report Upstream: | N/A | Work issues: | |
Branch: | fc4c596 (Commits, GitHub, GitLab) | Commit: | fc4c5961c379e547336288077b6bf1f46bb255c6 |
Dependencies: | Stopgaps: |
Description (last modified by )
This ticket implements a method to initialize a cone from vertices, rays, inequations and equalities.
More precisely
- we outsource a new method
_cone_from_normaliz_data
from the existing method_init_from_normaliz_data
. - Then we add a method
_cone_from_Vrepresentation_and_Hrepresentation
, which returns a cone formvertices
,rays
,ieqs
andeqns
.
Note that lines must be recomputed, hence the cone might reorder them.
The trivial cases of neither rays nor lines or without inequalities are not covered by this method. In either case None
is returned.
As a follow up we
Change History (14)
comment:1 Changed 3 years ago by
- Branch set to public/28639
- Commit set to c42c907f184264859fa6a5e37e523feb37568041
- Status changed from new to needs_info
comment:2 Changed 3 years ago by
I have set up doctests to test whether the cone is correct. Are there critical/important examples missing there?
comment:3 Changed 3 years ago by
- Description modified (diff)
comment:4 Changed 3 years ago by
I'm also a bit puzzled that it states that the equations are not precomputed from the data (see EXAMPLE block in the code). However, not giving the equations will lead to errors.
comment:5 Changed 3 years ago by
- Commit changed from c42c907f184264859fa6a5e37e523feb37568041 to cc17f0741bb323f06c6a65c5e0f08f1ea73a5655
Branch pushed to git repo; I updated commit sha1. New commits:
cc17f07 | added documentation to cone from normaliz data
|
comment:6 Changed 3 years ago by
- Description modified (diff)
- Summary changed from Polyhedron_normaliz initialize new cone from both Vrep and Hrep to Polyhedron_normaliz: Initialize new cone from both Vrep and Hrep
comment:7 Changed 3 years ago by
- Status changed from needs_info to needs_review
comment:8 Changed 3 years ago by
- Cc jipilab mkoeppe added
comment:9 Changed 3 years ago by
- Cc Winfried added
comment:10 follow-up: ↓ 12 Changed 3 years ago by
Minor detail:
- ``vertices`` -- list of point; each point can be specified - as any iterable container of - :meth:`~sage.geometry.polyhedron.base.base_ring` elements + as any iterable container of + :meth:`~sage.geometry.polyhedron.base.base_ring` elements
Also, shouldn't _cone_from_Vrepresentation_and_Hrepresentation
be called from somewhere in the code? Or is this done indirectly through the more general framework?
comment:11 Changed 3 years ago by
- Branch changed from public/28639 to public/28639-reb
- Commit changed from cc17f0741bb323f06c6a65c5e0f08f1ea73a5655 to fc4c5961c379e547336288077b6bf1f46bb255c6
comment:12 in reply to: ↑ 10 Changed 3 years ago by
I fixed the alignment error and also at the place, where I copied it from.
The method is not being used yet. I added a branch to #26363, which uses it in order to fix pickling/unpickling.
Initializing from both Vrep
and Hrep
does not work yet. Eventually, we should open a new ticket and allow it at least when converting to backend normaliz.
Replying to tscrim:
Minor detail:
- ``vertices`` -- list of point; each point can be specified - as any iterable container of - :meth:`~sage.geometry.polyhedron.base.base_ring` elements + as any iterable container of + :meth:`~sage.geometry.polyhedron.base.base_ring` elementsAlso, shouldn't
_cone_from_Vrepresentation_and_Hrepresentation
be called from somewhere in the code? Or is this done indirectly through the more general framework?
comment:13 Changed 3 years ago by
- Reviewers set to Travis Scrimshaw
- Status changed from needs_review to positive_review
Okay. Thanks. Then LGTM.
comment:14 Changed 3 years ago by
- Branch changed from public/28639-reb to fc4c5961c379e547336288077b6bf1f46bb255c6
- Resolution set to fixed
- Status changed from positive_review to closed
New commits:
method to obtain cone from Vrep and Hrep