Opened 17 months ago
Closed 17 months ago
#29229 closed enhancement (fixed)
Improvements for `is_reflexive` for polyhedra over the integers
Reported by:  ghkliem  Owned by:  

Priority:  major  Milestone:  sage9.1 
Component:  geometry  Keywords:  polytopes, is_reflexive 
Cc:  jipilab, ghLaisRast  Merged in:  
Authors:  Jonathan Kliem  Reviewers:  JeanPhilippe Labbé 
Report Upstream:  N/A  Work issues:  
Branch:  27fa75c (Commits, GitHub, GitLab)  Commit:  27fa75c21ba91bdae4204bb9c9e7f92b9cf75d1e 
Dependencies:  Stopgaps: 
Description (last modified by )
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 17 months ago by
 Branch set to public/29229
 Commit set to 1d7188951925228b0fbf9837542f153115b5cabd
 Keywords polytopes is_reflexive added
 Status changed from new to needs_review
comment:2 Changed 17 months ago by
 Description modified (diff)
comment:3 followup: ↓ 4 Changed 17 months ago by
 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 ; followup: ↓ 5 Changed 17 months ago by
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 17 months ago by
Replying to ghkliem:
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 17 months ago by
 Branch changed from public/29229 to public/29229reb
 Commit changed from 1d7188951925228b0fbf9837542f153115b5cabd to 0718db85f5ca8f73f06d6854965b210c9a500259
 Status changed from needs_work to needs_review
comment:7 Changed 17 months ago by
 Commit changed from 0718db85f5ca8f73f06d6854965b210c9a500259 to c2abdd6f3b1f55b46dbc2002c54f6a2f9808c942
Branch pushed to git repo; I updated commit sha1. New commits:
c2abdd6  typo in doc

comment:8 Changed 17 months ago by
 Reviewers set to JeanPhilippe 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 17 months ago by
 Status changed from needs_review to needs_work
comment:10 Changed 17 months ago by
 Commit changed from c2abdd6f3b1f55b46dbc2002c54f6a2f9808c942 to 27fa75c21ba91bdae4204bb9c9e7f92b9cf75d1e
Branch pushed to git repo; I updated commit sha1. New commits:
27fa75c  improved error message

comment:11 Changed 17 months ago by
 Status changed from needs_work to positive_review
comment:12 Changed 17 months ago by
 Branch changed from public/29229reb to 27fa75c21ba91bdae4204bb9c9e7f92b9cf75d1e
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
some improvements for `is_reflexive`