Opened 14 months ago
Closed 14 months ago
#31916 closed enhancement (fixed)
{Polyhedron, ConvexRationalPolyhedralCone}.{interior, relative_interior}
Reported by:  mkoeppe  Owned by:  

Priority:  major  Milestone:  sage9.4 
Component:  geometry  Keywords:  
Cc:  ghkliem, yzh, jipilab, tscrim, novoselt, mjo  Merged in:  
Authors:  Matthias Koeppe  Reviewers:  Jonathan Kliem 
Report Upstream:  N/A  Work issues:  
Branch:  fa4c2d2 (Commits, GitHub, GitLab)  Commit:  fa4c2d238a8f16a61f02e7cfa89ba3f17d8f4980 
Dependencies:  Stopgaps: 
Description (last modified by )
We introduce new methods relative_interior
and interior
, which (in nontrivial cases) return a simple object with a __contains__
method.
Then one can write x in P.relative_interior()
instead of P.relative_interior_contains(x)
.
This will also simplify #31660.
Change History (27)
comment:1 Changed 14 months ago by
comment:2 Changed 14 months ago by
 Branch set to u/mkoeppe/polyhedron_relative_interior
comment:3 Changed 14 months ago by
 Cc novoselt mjo added
 Commit set to e9c670c00a492f8f50f3e21e55895a0d3c1d55f2
 Description modified (diff)
 Summary changed from Polyhedron.relative_interior to {Polyhedron, ConvexRationalPolyhedralCone}.{interior, relative_interior}
comment:4 Changed 14 months ago by
 Commit changed from e9c670c00a492f8f50f3e21e55895a0d3c1d55f2 to b8bfe200f6e698dec855394af9adaaa63c282abd
comment:5 Changed 14 months ago by
 Commit changed from b8bfe200f6e698dec855394af9adaaa63c282abd to 6869673199fb80af0a09caf039873e27d50d2741
Branch pushed to git repo; I updated commit sha1. New commits:
6869673  relative_interior: Fix for dimension 0

comment:6 Changed 14 months ago by
 Commit changed from 6869673199fb80af0a09caf039873e27d50d2741 to 021d073e758d37f0abdb2466e3dc59e3383ed05b
Branch pushed to git repo; I updated commit sha1. New commits:
021d073  RelativeInterior: Add documentation, tests, comparison methods, method relative_interior

comment:7 Changed 14 months ago by
 Commit changed from 021d073e758d37f0abdb2466e3dc59e3383ed05b to 8f38e0475edfd5913bf25ede90162b04597db64c
Branch pushed to git repo; I updated commit sha1. New commits:
8f38e04  ConvexRationalPolyhedralCone.interior, relative_interior: Add doctests

comment:8 Changed 14 months ago by
 Status changed from new to needs_review
comment:9 followup: ↓ 12 Changed 14 months ago by
 Reviewers set to Jonathan Kliem
 Status changed from needs_review to needs_work
There are various small things:
The interior of a nonfulldimensional cone should be a cone, not a polyhedron.
Why do you implement relative_interior
for relative interiors but not interior
?
Why is closure
defined for relative interiors, but not for polyhedra or cones? The problem is, that P.relative_interior().closure()
will not always work.
Comparison will raise AttributeErrors
, when other
is the universe.
comment:10 Changed 14 months ago by
#31919 solves the problem with closure
.
comment:11 Changed 14 months ago by
 Commit changed from 8f38e0475edfd5913bf25ede90162b04597db64c to 5c089ec4926664d426e5d46735cb8c1a9b1d1017
comment:12 in reply to: ↑ 9 ; followup: ↓ 15 Changed 14 months ago by
Replying to ghkliem:
The interior of a nonfulldimensional cone should be a cone, not a polyhedron.
Can't do  a ConvexRationalPolyhedralCone
always contains 0
Why do you implement
relative_interior
for relative interiors but notinterior
?
Done
Why is
closure
defined for relative interiors, but not for polyhedra or cones? The problem is, thatP.relative_interior().closure()
will not always work.
For this ticket, it's just the accessor to the original polytope, not a more general API. See #31919 (ABC for convex sets), where I am adding such methods.
Comparison will raise
AttributeErrors
, whenother
is the universe.
Done
comment:13 Changed 14 months ago by
 Status changed from needs_work to needs_review
comment:15 in reply to: ↑ 12 Changed 14 months ago by
Replying to mkoeppe:
Replying to ghkliem:
The interior of a nonfulldimensional cone should be a cone, not a polyhedron.
Can't do  a
ConvexRationalPolyhedralCone
always contains 0
Right.
Why do you implement
relative_interior
for relative interiors but notinterior
?Done
Why is
closure
defined for relative interiors, but not for polyhedra or cones? The problem is, thatP.relative_interior().closure()
will not always work.For this ticket, it's just the accessor to the original polytope, not a more general API. See #31919 (ABC for convex sets), where I am adding such methods.
As noted above, I later discovered this on #31919 and it's fine to add it later on.
Comparison will raise
AttributeErrors
, whenother
is the universe.Done
comment:16 Changed 14 months ago by
 Status changed from positive_review to needs_work
Thanks! But I think I need to revise relative_interior
to handle polyhedra that are already relatively open
comment:17 Changed 14 months ago by
 Commit changed from 5c089ec4926664d426e5d46735cb8c1a9b1d1017 to 86ce301c02936f4b56f6ab821520e8542bd5fbc1
Branch pushed to git repo; I updated commit sha1. New commits:
86ce301  Polyhedron_base.is_relatively_open: New; fix relative_interior for affine subspaces

comment:18 Changed 14 months ago by
 Status changed from needs_work to needs_review
comment:19 Changed 14 months ago by
 Status changed from needs_review to positive_review
I was locking for a boundary case, but yes, if a polyhedron is it's affine hull, then it either empty or the universe it it's affine hull.
comment:20 Changed 14 months ago by
 Commit changed from 86ce301c02936f4b56f6ab821520e8542bd5fbc1 to 216cb811d16bdf1bedfb2d7e04339ed4387f7212
 Status changed from positive_review to needs_review
Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:
216cb81  ConvexRationalPolyhedralCone.is_relatively_open: New, fix ConvexRationalPolyhedralCone.relative_interior for linear subspaces

comment:21 Changed 14 months ago by
Sorry, here's the same fix for cones
comment:22 Changed 14 months ago by
 Commit changed from 216cb811d16bdf1bedfb2d7e04339ed4387f7212 to 44cde1e3dbea9086e75f235c0f61164278e4e451
Branch pushed to git repo; I updated commit sha1. New commits:
44cde1e  src/doc/en/reference/discrete_geometry/index.rst: Add sage/geometry/relative_interior

comment:23 Changed 14 months ago by
Is there a reason for the emptylines:
+ def is_relatively_open(self): + + r""" + Return whether ``self`` is relatively open.
+class RelativeInterior(SageObject): + + r"""
Surround toplevel function and class definitions with two blank lines.
+# **************************************************************************** + +from sage.structure.sage_object import SageObject + +class RelativeInterior(SageObject):
I'm not sure, if it also applies to the import.
Really tiny: Does a docstring ending with an example contain an empty line in the end? (Doesn't need to be fixed, I just noticed.)
Maybe for another ticket: Replace the ppl
imports in cone.py
by explicity imports to help pyflakes.
Otherwise it's a positive review.
comment:24 Changed 14 months ago by
 Commit changed from 44cde1e3dbea9086e75f235c0f61164278e4e451 to fa4c2d238a8f16a61f02e7cfa89ba3f17d8f4980
Branch pushed to git repo; I updated commit sha1. New commits:
fa4c2d2  Whitespace fixes

comment:26 Changed 14 months ago by
Thanks!
comment:27 Changed 14 months ago by
 Branch changed from u/mkoeppe/polyhedron_relative_interior to fa4c2d238a8f16a61f02e7cfa89ba3f17d8f4980
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
sage.geometry.polyhedron.relint, Polyhedron_base.relative_interior, Polyhedron_base.interior: New