Opened 2 years ago
Closed 2 years ago
#28639 closed enhancement (fixed)
Polyhedron_normaliz: Initialize new cone from both Vrep and Hrep
Reported by:  ghkliem  Owned by:  

Priority:  major  Milestone:  sage9.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 2 years ago by
 Branch set to public/28639
 Commit set to c42c907f184264859fa6a5e37e523feb37568041
 Status changed from new to needs_info
comment:2 Changed 2 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 2 years ago by
 Description modified (diff)
comment:4 Changed 2 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 2 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 2 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 2 years ago by
 Status changed from needs_info to needs_review
comment:8 Changed 2 years ago by
 Cc jipilab mkoeppe added
comment:9 Changed 2 years ago by
 Cc Winfried added
comment:10 followup: ↓ 12 Changed 2 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 2 years ago by
 Branch changed from public/28639 to public/28639reb
 Commit changed from cc17f0741bb323f06c6a65c5e0f08f1ea73a5655 to fc4c5961c379e547336288077b6bf1f46bb255c6
comment:12 in reply to: ↑ 10 Changed 2 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 2 years ago by
 Reviewers set to Travis Scrimshaw
 Status changed from needs_review to positive_review
Okay. Thanks. Then LGTM.
comment:14 Changed 2 years ago by
 Branch changed from public/28639reb to fc4c5961c379e547336288077b6bf1f46bb255c6
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
method to obtain cone from Vrep and Hrep