#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:

Status badges

Description (last modified by gh-kliem)

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 form vertices, rays, ieqs and eqns.

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

  • fix Polyhedron_normaliz.save in #26363,
  • we allow converting to 'normaliz' backend with both Vrep and Hrep in the spirit of #22701.

Change History (14)

comment:1 Changed 18 months ago by gh-kliem

  • Branch set to public/28639
  • Commit set to c42c907f184264859fa6a5e37e523feb37568041
  • Status changed from new to needs_info

New commits:

c42c907method to obtain cone from Vrep and Hrep

comment:2 Changed 18 months ago by gh-kliem

I have set up doctests to test whether the cone is correct. Are there critical/important examples missing there?

comment:3 Changed 18 months ago by gh-kliem

  • Description modified (diff)

comment:4 Changed 18 months ago by gh-kliem

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 18 months ago by git

  • Commit changed from c42c907f184264859fa6a5e37e523feb37568041 to cc17f0741bb323f06c6a65c5e0f08f1ea73a5655

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

cc17f07added documentation to cone from normaliz data

comment:6 Changed 18 months ago by gh-kliem

  • 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 18 months ago by gh-kliem

  • Status changed from needs_info to needs_review

comment:8 Changed 18 months ago by jipilab

  • Cc jipilab mkoeppe added

comment:9 Changed 17 months ago by gh-kliem

  • Cc Winfried added

comment:10 follow-up: Changed 17 months ago by 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` 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 17 months ago by gh-kliem

  • Branch changed from public/28639 to public/28639-reb
  • Commit changed from cc17f0741bb323f06c6a65c5e0f08f1ea73a5655 to fc4c5961c379e547336288077b6bf1f46bb255c6

New commits:

a54cfd9Merge branch 'public/28639' of git://trac.sagemath.org/sage into public/28639-reb
fc4c596alignment fix in docs

comment:12 in reply to: ↑ 10 Changed 17 months ago by gh-kliem

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` 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:13 Changed 17 months ago by tscrim

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

Okay. Thanks. Then LGTM.

comment:14 Changed 17 months ago by vbraun

  • Branch changed from public/28639-reb to fc4c5961c379e547336288077b6bf1f46bb255c6
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.