#29229 closed enhancement (fixed)

Improvements for `is_reflexive` for polyhedra over the integers

Reported by: gh-kliem Owned by:
Priority: major Milestone: sage-9.1
Component: geometry Keywords: polytopes, is_reflexive
Cc: jipilab, gh-LaisRast Merged in:
Authors: Jonathan Kliem Reviewers: Jean-Philippe Labbé
Report Upstream: N/A Work issues:
Branch: 27fa75c (Commits, GitHub, GitLab) Commit: 27fa75c21ba91bdae4204bb9c9e7f92b9cf75d1e
Dependencies: Stopgaps:

Status badges

Description (last modified by gh-kliem)

Currently, this method has almost no documentation.

Also it relies on polar to work differently for Polyhedron_ZZ than for Polyhedron_base. However, this is not being tested at all. We fix this by checking being reflexive just from the inequalities.

This also improves the error messages. We raise a ValueError if the polyhedron is unbounded. If the polytope does not have the origin in the interior, we return False, as this is clearly not reflexive.

Change History (12)

comment:1 Changed 14 months ago by gh-kliem

  • Branch set to public/29229
  • Commit set to 1d7188951925228b0fbf9837542f153115b5cabd
  • Keywords polytopes is_reflexive added
  • Status changed from new to needs_review

New commits:

1d71889some improvements for `is_reflexive`

comment:2 Changed 14 months ago by gh-kliem

  • Description modified (diff)

comment:3 follow-up: Changed 14 months ago by jipilab

  • Status changed from needs_review to needs_work

Some remarks:

-        A lattice polytope is reflexive if it is polar to a lattice polytope.
+        A lattice polytope is reflexive if its polar is a lattice polytope.

Even though the raised error is not wrong as written (that it is not a polytope), I would still say "the polyhedron is not compact" or "the polyhedron should be compact". Simply because this avoids an eventual confusion in nomenclature polytope vs polyhedron.

comment:4 in reply to: ↑ 3 ; follow-up: Changed 14 months ago by gh-kliem

Replying to jipilab:

Some remarks:

-        A lattice polytope is reflexive if it is polar to a lattice polytope.
+        A lattice polytope is reflexive if its polar is a lattice polytope.

Those two statements are not equivalent. I would say that a polytope without origin in interior is not reflexive and that the return value shall be False (and no error message). However, in this case the polar is not defined. This is why I chose this specific formulation.

Even though the raised error is not wrong as written (that it is not a polytope), I would still say "the polyhedron is not compact" or "the polyhedron should be compact". Simply because this avoids an eventual confusion in nomenclature polytope vs polyhedron.

comment:5 in reply to: ↑ 4 Changed 14 months ago by jipilab

Replying to gh-kliem:

Replying to jipilab:

Some remarks:

-        A lattice polytope is reflexive if it is polar to a lattice polytope.
+        A lattice polytope is reflexive if its polar is a lattice polytope.

Those two statements are not equivalent. I would say that a polytope without origin in interior is not reflexive and that the return value shall be False (and no error message). However, in this case the polar is not defined. This is why I chose this specific formulation.

Then, one should be transparent. Why not something like:

+       A lattice polytope is reflexive if it contains the origin and its polar with respect to the origin is a lattice polytope.

comment:6 Changed 14 months ago by gh-kliem

  • Branch changed from public/29229 to public/29229-reb
  • Commit changed from 1d7188951925228b0fbf9837542f153115b5cabd to 0718db85f5ca8f73f06d6854965b210c9a500259
  • Status changed from needs_work to needs_review

New commits:

1930179some improvements for `is_reflexive`
0718db8small improvments

comment:7 Changed 14 months ago by git

  • Commit changed from 0718db85f5ca8f73f06d6854965b210c9a500259 to c2abdd6f3b1f55b46dbc2002c54f6a2f9808c942

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

c2abdd6typo in doc

comment:8 Changed 13 months ago by jipilab

  • Reviewers set to Jean-Philippe Labbé

One last things:

- "the polyhedron should be compact"
+ "the polyhedron is not compact"

sounds better to me. A "should" gives it too many ways to be interpreted, whereas a "is not" is only reporting a fact, which I prefer. Not a big thing...

Patchbots look okay to me. So you can put this on positive review once you make this change on my behalf.

comment:9 Changed 13 months ago by jipilab

  • Status changed from needs_review to needs_work

comment:10 Changed 13 months ago by git

  • Commit changed from c2abdd6f3b1f55b46dbc2002c54f6a2f9808c942 to 27fa75c21ba91bdae4204bb9c9e7f92b9cf75d1e

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

27fa75cimproved error message

comment:11 Changed 13 months ago by gh-kliem

  • Status changed from needs_work to positive_review

comment:12 Changed 13 months ago by vbraun

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