Opened 2 years ago
Closed 2 years ago
#28604 closed enhancement (fixed)
CombinatorialPolyhedron: ridge_graph > facet_graph
Reported by:  ghkliem  Owned by:  

Priority:  major  Milestone:  sage9.0 
Component:  geometry  Keywords:  polytopes, combinatorial polyhedron 
Cc:  jipilab, ghLaisRast  Merged in:  
Authors:  Jonathan Kliem  Reviewers:  Laith Rastanawi 
Report Upstream:  N/A  Work issues:  
Branch:  9d93a14 (Commits, GitHub, GitLab)  Commit:  9d93a14cfeb780aa3fe4c540edb8f895fc098ef9 
Dependencies:  #28603  Stopgaps: 
Description (last modified by )
In order to make CombinatorialPolyhedron
more consistent with Polyhedron
, we replace ridge_graph
by facet_graph
.
In case of of the half space this makes a difference, as there is a facet, but no ridges.
For now we keep ridge_graph
and add a deprecation warning.
While we are at it, we fix some bugs:
CombinatorialPolyhedron.ridges(names=False, add_equalities=True)
ignoresadd_equalities=True
now.
Equalities do not carry indices and can only be added with
names=True
.
facet_graph
is now aware of the following: a ridge inridges
is a tuple of indices withnames=False
and a tuple of tuples withnames=True
andadd_equalities=True
,
e.g.
(1,2)
vs.(('my_vertex_1',), ('my_vertex_2',))
.
ridges
now appends equations at the end. This is consistent withFaceIterator.Hrepresentation
.
Change History (14)
comment:1 Changed 2 years ago by
 Branch set to public/28604
 Cc jipilab ghLaisRast added
 Commit set to 8654f9ff547dfefa6042da0b02c486e0e81d7717
 Description modified (diff)
 Status changed from new to needs_review
comment:2 Changed 2 years ago by
 Status changed from needs_review to needs_work
facet_graph
does not work for nonfulldimensional polyhedra:
sage: C = CombinatorialPolyhedron(polytopes.hypersimplex(5,2)); C A 4dimensional combinatorial polyhedron with 10 facets sage: C.facet_graph() Graph on 20 vertices
The problem arises since each facet in C.ridges(names=True, add_equalities=True)
is represented as (equation, inequality), while in C.face_iter(C.dimension()  1, dual=False))
, each facet is represented as (inequality, equation).
facet_graph
does not work when names=False
:
sage: C = CombinatorialPolyhedron(polytopes.cube()); C A 3dimensional combinatorial polyhedron with 6 facets sage: C.facet_graph(names=False) Graph on 12 vertices
The problem arises sincefacet.Hrepr(names=False)
returns a tuple, for example (5,)
. While in C.ridges(names=False)
, each facet is represented by a number.
comment:3 Changed 2 years ago by
 Commit changed from 8654f9ff547dfefa6042da0b02c486e0e81d7717 to 597afd89cb64f59afabb939129d381f0d2b1d693
comment:4 Changed 2 years ago by
 Description modified (diff)
 Status changed from needs_work to needs_review
comment:5 Changed 2 years ago by
 Description modified (diff)
comment:6 Changed 2 years ago by
 Status changed from needs_review to positive_review
After the changes, I think that the ticket is good and thus I will put it on "positive review".
comment:7 Changed 2 years ago by
 Reviewers set to Laith Rastanawi
comment:8 Changed 2 years ago by
 Commit changed from 597afd89cb64f59afabb939129d381f0d2b1d693 to 3a6f44f9eaa66a79072d2b0117595df17b32ece2
 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:
3a6f44f  improved deprecation warning

comment:9 Changed 2 years ago by
 Status changed from needs_review to positive_review
comment:10 Changed 2 years ago by
 Commit changed from 3a6f44f9eaa66a79072d2b0117595df17b32ece2 to bcb754fd8272c3fc5046baa5b5191a6afc3ef40c
 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:
bcb754f  deprecation warning gives new name

comment:11 Changed 2 years ago by
 Commit changed from bcb754fd8272c3fc5046baa5b5191a6afc3ef40c to 9d93a14cfeb780aa3fe4c540edb8f895fc098ef9
Branch pushed to git repo; I updated commit sha1. New commits:
9d93a14  changed stacklevel to 3, so deprecation warning shows during normal usage

comment:12 Changed 2 years ago by
 Status changed from needs_review to positive_review
The deprecation warning is now printed out. So I will put it back on "positive review".
comment:13 Changed 2 years ago by
 Description modified (diff)
comment:14 Changed 2 years ago by
 Branch changed from public/28604 to 9d93a14cfeb780aa3fe4c540edb8f895fc098ef9
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
added vertex_graph, deprecated edge_graph
ridge_graph > facet_graph; fixed bug in ridges with arguments