Skip to content

Commit

Permalink
Trac #28614: CombinatorialPolyhedron: length_* to n_*
Browse files Browse the repository at this point in the history
To make `CombinatorialPolyhedron` more consistent with `Polyhedron` we
change the following names.

- `length_Hrepr` -> `n_Hrepresentation`
- `length_Vrepr` -> `n_Vrepresentation`

In `FaceIterator`:
- `length_atom_repr` -> `n_atom_rep` (note that #28608 changes `repr` to
`rep`)

In `CombinatorialFace`:
- `length_Vrepr` -> `n_ambient_Vrepresentation`
- `length_Hrepr` -> `n_ambient_Hrepresentation`
As both methods are public we keep the old methods with deprecation
warnings.

As a follow up in #28615 we need to fix the alignment in
`src/sage/geometry/polyhedron/combinatorial_polyhedron/base.pxd`. Fixing
it in this ticket would lead to merge conflicts and as its trivial, we
can easily do it later.

URL: https://trac.sagemath.org/28614
Reported by: gh-kliem
Ticket author(s): Jonathan Kliem
Reviewer(s): Frédéric Chapoton
  • Loading branch information
Release Manager committed Nov 28, 2019
2 parents d8818f1 + 2ad0ec0 commit efe7f55
Show file tree
Hide file tree
Showing 8 changed files with 97 additions and 54 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ cdef class CombinatorialPolyhedron(SageObject):
cdef tuple _H # the names of HRep, if they exist
cdef tuple _equalities # stores equalities, given on input (might belong to Hrep)
cdef int _dimension # stores dimension, -2 on init
cdef unsigned int _length_Hrepr # Hrepr might include equalities
cdef unsigned int _length_Vrepr # Vrepr might include rays/lines
cdef unsigned int _n_Hrepresentation # Hrepr might include equalities
cdef unsigned int _n_Vrepresentation # Vrepr might include rays/lines
cdef size_t _n_facets # length Hrep without equalities
cdef bint _bounded # ``True`` iff Polyhedron is bounded
cdef ListOfFaces _bitrep_facets # facets in bit representation
Expand Down Expand Up @@ -43,8 +43,8 @@ cdef class CombinatorialPolyhedron(SageObject):
cdef tuple V(self)
cdef tuple H(self)
cdef tuple equalities(self)
cdef unsigned int length_Vrepr(self)
cdef unsigned int length_Hrepr(self)
cdef unsigned int n_Vrepresentation(self)
cdef unsigned int n_Hrepresentation(self)
cdef bint is_bounded(self)
cdef ListOfFaces bitrep_facets(self)
cdef ListOfFaces bitrep_Vrepr(self)
Expand Down
57 changes: 28 additions & 29 deletions src/sage/geometry/polyhedron/combinatorial_polyhedron/base.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ AUTHOR:
# https://www.gnu.org/licenses/
# ****************************************************************************

from __future__ import absolute_import, division, print_function
import numbers
from sage.rings.integer import Integer
from sage.graphs.graph import Graph
Expand Down Expand Up @@ -333,9 +332,9 @@ cdef class CombinatorialPolyhedron(SageObject):
# input is ``LatticePolytope``
self._bounded = True
Vrepr = data.vertices()
self._length_Vrepr = len(Vrepr)
self._n_Vrepresentation = len(Vrepr)
facets = data.facets()
self._length_Hrepr = len(facets)
self._n_Hrepresentation = len(facets)
data = tuple(tuple(vert for vert in facet.vertices())
for facet in facets)
else:
Expand Down Expand Up @@ -380,8 +379,8 @@ cdef class CombinatorialPolyhedron(SageObject):

if isinstance(data, Matrix):
# Input is incidence-matrix or was converted to it.
self._length_Hrepr = data.ncols()
self._length_Vrepr = data.nrows()
self._n_Hrepresentation = data.ncols()
self._n_Vrepresentation = data.nrows()

# Initializing the facets in their Bit-representation.
self._bitrep_facets = incidence_matrix_to_bit_repr_of_facets(data)
Expand All @@ -393,7 +392,7 @@ cdef class CombinatorialPolyhedron(SageObject):

# Initialize far_face if unbounded.
if not self._bounded:
self._far_face = facets_tuple_to_bit_repr_of_facets((tuple(far_face),), self._length_Vrepr)
self._far_face = facets_tuple_to_bit_repr_of_facets((tuple(far_face),), self._n_Vrepresentation)
else:
self._far_face = None

Expand Down Expand Up @@ -423,16 +422,16 @@ cdef class CombinatorialPolyhedron(SageObject):
if self._V is None:
# Get the names of the Vrepr.
Vrepr = sorted(set.union(*map(set, data)))
length_Vrepr = len(Vrepr)
n_Vrepresentation = len(Vrepr)
if Vrepr != range(len(Vrepr)):
self._V = tuple(Vrepr)
Vinv = {v: i for i,v in enumerate(self._V)}
else:
# Assuming the user gave as correct names for the vertices
# and labeled them instead by `0,...,n`.
length_Vrepr = len(self._V)
n_Vrepresentation = len(self._V)

self._length_Vrepr = length_Vrepr
self._n_Vrepresentation = n_Vrepresentation

# Relabel the Vrepr to be `0,...,n`.
if self._V is not None:
Expand All @@ -442,17 +441,17 @@ cdef class CombinatorialPolyhedron(SageObject):
facets = tuple(tuple(f(i) for i in j) for j in data)

self._n_facets = len(facets)
self._length_Hrepr = len(facets)
self._n_Hrepresentation = len(facets)

# Initializing the facets in their Bit-representation.
self._bitrep_facets = facets_tuple_to_bit_repr_of_facets(facets, length_Vrepr)
self._bitrep_facets = facets_tuple_to_bit_repr_of_facets(facets, n_Vrepresentation)

# Initializing the Vrepr as their Bit-representation.
self._bitrep_Vrepr = facets_tuple_to_bit_repr_of_Vrepr(facets, length_Vrepr)
self._bitrep_Vrepr = facets_tuple_to_bit_repr_of_Vrepr(facets, n_Vrepresentation)

# Initialize far_face if unbounded.
if not self._bounded:
self._far_face = facets_tuple_to_bit_repr_of_facets((tuple(far_face),), length_Vrepr)
self._far_face = facets_tuple_to_bit_repr_of_facets((tuple(far_face),), n_Vrepresentation)
else:
self._far_face = None

Expand Down Expand Up @@ -572,7 +571,7 @@ cdef class CombinatorialPolyhedron(SageObject):
if self.V() is not None:
return self.V()
else:
return tuple(smallInteger(i) for i in range(self.length_Vrepr()))
return tuple(smallInteger(i) for i in range(self.n_Vrepresentation()))

def Hrepresentation(self):
r"""
Expand All @@ -594,7 +593,7 @@ cdef class CombinatorialPolyhedron(SageObject):
if self.H() is not None:
return self.equalities() + self.H()
else:
return tuple(smallInteger(i) for i in range(self.length_Hrepr()))
return tuple(smallInteger(i) for i in range(self.n_Hrepresentation()))

def dimension(self):
r"""
Expand All @@ -617,8 +616,8 @@ cdef class CombinatorialPolyhedron(SageObject):
# The dimension of a trivial polyhedron is assumed to contain
# exactly one "vertex" and for each dimension one "line" as in
# :class:`~sage.geometry.polyhedron.parent.Polyhedron_base`
self._dimension = self.length_Vrepr() - 1
elif not self.is_bounded() or self.n_facets() <= self.length_Vrepr():
self._dimension = self.n_Vrepresentation() - 1
elif not self.is_bounded() or self.n_facets() <= self.n_Vrepresentation():
self._dimension = self.bitrep_facets().compute_dimension()
else:
# If the polyhedron has many facets,
Expand Down Expand Up @@ -674,7 +673,7 @@ cdef class CombinatorialPolyhedron(SageObject):
# Some elements in the ``Vrepr`` might not correspond to actual combinatorial vertices.
return len(self.vertices())
else:
return smallInteger(self.length_Vrepr())
return smallInteger(self.n_Vrepresentation())

def vertices(self, names=True):
r"""
Expand Down Expand Up @@ -741,9 +740,9 @@ cdef class CombinatorialPolyhedron(SageObject):
# The Polyhedron has no vertex.
return ()
if names and self.V():
return tuple(self.V()[i] for i in range(self.length_Vrepr()) if not i in self.far_face_tuple())
return tuple(self.V()[i] for i in range(self.n_Vrepresentation()) if not i in self.far_face_tuple())
else:
return tuple(smallInteger(i) for i in range(self.length_Vrepr()) if not i in self.far_face_tuple())
return tuple(smallInteger(i) for i in range(self.n_Vrepresentation()) if not i in self.far_face_tuple())

def n_facets(self):
r"""
Expand Down Expand Up @@ -911,7 +910,7 @@ cdef class CombinatorialPolyhedron(SageObject):
# compute the edges.
if not self.is_bounded():
self._compute_edges(dual=False)
elif self.length_Vrepr() > self.n_facets()*self.n_facets():
elif self.n_Vrepresentation() > self.n_facets()*self.n_facets():
# This is a wild estimate
# that in this case it is better not to use the dual.
self._compute_edges(dual=False)
Expand Down Expand Up @@ -1093,7 +1092,7 @@ cdef class CombinatorialPolyhedron(SageObject):
# compute the ridges.
if not self.is_bounded():
self._compute_ridges(dual=False)
elif self.length_Vrepr()*self.length_Vrepr() < self.n_facets():
elif self.n_Vrepresentation()*self.n_Vrepresentation() < self.n_facets():
# This is a wild estimate
# that in this case it is better to use the dual.
self._compute_edges(dual=True)
Expand Down Expand Up @@ -1326,7 +1325,7 @@ cdef class CombinatorialPolyhedron(SageObject):
cdef FaceIterator face_iter
if dual is None:
# Determine the faster way, to iterate through all faces.
if not self.is_bounded() or self.n_facets() <= self.length_Vrepr():
if not self.is_bounded() or self.n_facets() <= self.n_Vrepresentation():
dual = False
else:
dual = True
Expand Down Expand Up @@ -1560,17 +1559,17 @@ cdef class CombinatorialPolyhedron(SageObject):
"""
return self._equalities

cdef unsigned int length_Vrepr(self):
cdef unsigned int n_Vrepresentation(self):
r"""
Return the number of elements in the Vrepresentation.
"""
return self._length_Vrepr
return self._n_Vrepresentation

cdef unsigned int length_Hrepr(self):
cdef unsigned int n_Hrepresentation(self):
r"""
Return the number of elements in the Hrepresentation.
"""
return self._length_Hrepr
return self._n_Hrepresentation

cdef bint is_bounded(self):
r"""
Expand Down Expand Up @@ -1614,7 +1613,7 @@ cdef class CombinatorialPolyhedron(SageObject):
return 0 # There is no need to recompute the f_vector.

cdef bint dual
if not self.is_bounded() or self.n_facets() <= self.length_Vrepr():
if not self.is_bounded() or self.n_facets() <= self.n_Vrepresentation():
# In this case the non-dual approach is faster..
dual = False
else:
Expand Down Expand Up @@ -1653,7 +1652,7 @@ cdef class CombinatorialPolyhedron(SageObject):

else:
if self.is_bounded() and dim > 1 \
and f_vector[1] < self.length_Vrepr() - len(self.far_face_tuple()):
and f_vector[1] < self.n_Vrepresentation() - len(self.far_face_tuple()):
# The input seemed to be wrong.
raise ValueError("not all vertices are intersections of facets")

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,6 @@ cdef class CombinatorialFace(SageObject):
# If ``dual == 0``, then coatoms are facets, atoms vertices and vice versa.
cdef ListOfFaces atoms, coatoms

cdef size_t length_atom_repr(self) except -1
cdef size_t n_atom_rep(self) except -1
cdef size_t set_coatom_repr(self) except -1
cdef size_t set_atom_repr(self) except -1
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ Combinatorial face of a polyhedron
This module provides the combinatorial type of a polyhedral face.
,, SEEALSO::
.. SEEALSO::
:mod:`sage.geometry.polyhedron.combinatorial_polyhedron.base`,
:mod:`sage.geometry.polyhedron.combinatorial_polyhedron.face_iterator`.
Expand Down Expand Up @@ -36,7 +36,7 @@ Obtain further information regarding a face::
A 2-dimensional face of a 3-dimensional combinatorial polyhedron
sage: face.Vrepr()
(A vertex at (0, 0, 1), A vertex at (0, 1, 0), A vertex at (1, 0, 0))
sage: face.length_Vrepr()
sage: face.n_ambient_Vrepresentation()
3
sage: face.Hrepr(names=False)
(5,)
Expand Down Expand Up @@ -64,6 +64,8 @@ AUTHOR:
# http://www.gnu.org/licenses/
#*****************************************************************************

from sage.misc.superseded import deprecated_function_alias

import numbers
from sage.rings.integer cimport smallInteger
from .conversions cimport bit_repr_to_Vrepr_list
Expand Down Expand Up @@ -114,7 +116,7 @@ cdef class CombinatorialFace(SageObject):
(A vertex at (6, 36, 216, 1296, 7776),)
sage: face.Vrepr(names=False)
(6,)
sage: face.length_Vrepr()
sage: face.n_ambient_Vrepresentation()
1
The Hrepresentation::
Expand All @@ -133,7 +135,7 @@ cdef class CombinatorialFace(SageObject):
An inequality (-210, 317, -125, 19, -1) x + 0 >= 0)
sage: face.Hrepr(names=False)
(3, 4, 5, 6, 7, 8, 9, 10, 11, 18, 19)
sage: face.length_Hrepr()
sage: face.n_ambient_Hrepresentation()
11
"""
def __init__(self, data, dimension=None, index=None):
Expand Down Expand Up @@ -381,7 +383,7 @@ cdef class CombinatorialFace(SageObject):
return tuple(smallInteger(self.atom_repr[i])
for i in range(length))

def length_Vrepr(self):
def n_ambient_Vrepresentation(self):
r"""
Return the length of the face.
Expand All @@ -392,13 +394,25 @@ cdef class CombinatorialFace(SageObject):
sage: P = polytopes.cube()
sage: C = CombinatorialPolyhedron(P)
sage: it = C.face_iter()
sage: all(face.length_Vrepr() == len(face.Vrepr()) for face in it)
sage: all(face.n_ambient_Vrepresentation() == len(face.Vrepr()) for face in it)
True
TESTS::
sage: P = polytopes.cube()
sage: C = CombinatorialPolyhedron(P)
sage: it = C.face_iter()
sage: face = next(it)
sage: _ = face.n_Vrepr()
doctest:...: DeprecationWarning: n_Vrepr is deprecated. Please use n_ambient_Vrepresentation instead.
See https://trac.sagemath.org/28614 for details.
"""
if self._dual:
return smallInteger(self.set_coatom_repr())
else:
return smallInteger(self.length_atom_repr())
return smallInteger(self.n_atom_rep())

n_Vrepr = deprecated_function_alias(28614, n_ambient_Vrepresentation)

def Hrepr(self, names=True):
r"""
Expand Down Expand Up @@ -488,7 +502,7 @@ cdef class CombinatorialFace(SageObject):
return tuple(smallInteger(self.atom_repr[i])
for i in range(length))

def length_Hrepr(self):
def n_ambient_Hrepresentation(self):
r"""
Returns the length of the :meth:`Hrepr`.
Expand All @@ -499,15 +513,47 @@ cdef class CombinatorialFace(SageObject):
sage: P = polytopes.cube()
sage: C = CombinatorialPolyhedron(P)
sage: it = C.face_iter()
sage: all(face.length_Hrepr() == len(face.Hrepr()) for face in it)
sage: all(face.n_ambient_Hrepresentation() == len(face.Hrepr()) for face in it)
True
TESTS::
sage: P = polytopes.cube()
sage: C = CombinatorialPolyhedron(P)
sage: it = C.face_iter()
sage: face = next(it)
sage: _ = face.n_Hrepr()
doctest:...: DeprecationWarning: n_Hrepr is deprecated. Please use n_ambient_Hrepresentation instead.
See https://trac.sagemath.org/28614 for details.
"""
if not self._dual:
return smallInteger(self.set_coatom_repr())
else:
return smallInteger(self.length_atom_repr())
return smallInteger(self.n_atom_rep())

n_Hrepr = deprecated_function_alias(28614, n_ambient_Hrepresentation)

def n_Hrepr(self):
r"""
.. SEEALSO::
:meth:`CombinatorialFace.n_ambient_Hrepresentation`
TESTS::
sage: P = polytopes.cube()
sage: C = CombinatorialPolyhedron(P)
sage: it = C.face_iter()
sage: face = next(it)
sage: _ = face.n_Hrepr()
doctest:...: DeprecationWarning: n_Hrepr is deprecated. Please use n_ambient_Hrepresentation instead.
See https://trac.sagemath.org/28614 for details.
"""
from sage.misc.superseded import deprecation
deprecation(28614, "the method n_Hrepr of CombinatorialFace is deprecated")
return self.n_ambient_Hrepresentation()

cdef size_t length_atom_repr(self) except -1:
cdef size_t n_atom_rep(self) except -1:
r"""
Compute the number of atoms in the current face by counting the
number of set bits.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ AUTHOR:
# http://www.gnu.org/licenses/
#*****************************************************************************

from __future__ import absolute_import, division
from sage.structure.element import is_Matrix

from libc.string cimport memset
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,6 @@ cdef class FaceIterator(SageObject):
cdef inline CombinatorialFace next_face(self)
cdef inline int next_dimension(self) except -1
cdef inline int next_face_loop(self) except -1
cdef size_t length_atom_repr(self) except -1
cdef size_t n_atom_rep(self) except -1
cdef size_t set_coatom_repr(self) except -1
cdef size_t set_atom_repr(self) except -1
Loading

0 comments on commit efe7f55

Please sign in to comment.