Opened 2 years ago

Closed 2 years ago

#28604 closed enhancement (fixed)

CombinatorialPolyhedron: ridge_graph -> facet_graph

Reported by: gh-kliem Owned by:
Priority: major Milestone: sage-9.0
Component: geometry Keywords: polytopes, combinatorial polyhedron
Cc: jipilab, gh-LaisRast Merged in:
Authors: Jonathan Kliem Reviewers: Laith Rastanawi
Report Upstream: N/A Work issues:
Branch: 9d93a14 (Commits, GitHub, GitLab) Commit: 9d93a14cfeb780aa3fe4c540edb8f895fc098ef9
Dependencies: #28603 Stopgaps:

Status badges

Description (last modified by gh-kliem)

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) ignores add_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 in ridges is a tuple of indices with names=False and a tuple of tuples with names=True and add_equalities=True,

e.g. (1,2) vs. (('my_vertex_1',), ('my_vertex_2',)).

  • ridges now appends equations at the end. This is consistent with FaceIterator.Hrepresentation.

Change History (14)

comment:1 Changed 2 years ago by gh-kliem

  • Authors set to Jonathan Kliem
  • Branch set to public/28604
  • Cc jipilab gh-LaisRast added
  • Commit set to 8654f9ff547dfefa6042da0b02c486e0e81d7717
  • Description modified (diff)
  • Status changed from new to needs_review

New commits:

a005e47added vertex_graph, deprecated edge_graph
8654f9fridge_graph -> facet_graph; fixed bug in ridges with arguments

comment:2 Changed 2 years ago by gh-LaisRast

  • Status changed from needs_review to needs_work

facet_graph does not work for non-full-dimensional polyhedra:

    sage: C = CombinatorialPolyhedron(polytopes.hypersimplex(5,2)); C
    A 4-dimensional 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 3-dimensional 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 git

  • Commit changed from 8654f9ff547dfefa6042da0b02c486e0e81d7717 to 597afd89cb64f59afabb939129d381f0d2b1d693

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

c54886bbux fix regarding elements and 1-element tuples
597afd8ridges append eualities at the end

comment:4 Changed 2 years ago by gh-kliem

  • Description modified (diff)
  • Status changed from needs_work to needs_review

comment:5 Changed 2 years ago by gh-kliem

  • Description modified (diff)

comment:6 Changed 2 years ago by gh-LaisRast

  • 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 gh-LaisRast

  • Reviewers set to Laith Rastanawi

comment:8 Changed 2 years ago by git

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

3a6f44fimproved deprecation warning

comment:9 Changed 2 years ago by gh-LaisRast

  • Status changed from needs_review to positive_review

comment:10 Changed 2 years ago by git

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

bcb754fdeprecation warning gives new name

comment:11 Changed 2 years ago by git

  • Commit changed from bcb754fd8272c3fc5046baa5b5191a6afc3ef40c to 9d93a14cfeb780aa3fe4c540edb8f895fc098ef9

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

9d93a14changed stacklevel to 3, so deprecation warning shows during normal usage

comment:12 Changed 2 years ago by gh-LaisRast

  • 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 gh-kliem

  • Description modified (diff)

comment:14 Changed 2 years ago by vbraun

  • Branch changed from public/28604 to 9d93a14cfeb780aa3fe4c540edb8f895fc098ef9
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.