From dd0729d8efcbe8a5160f1ea1e82d86203d16cb66 Mon Sep 17 00:00:00 2001 From: Jonathan Kliem Date: Mon, 28 Oct 2019 11:50:51 +0100 Subject: [PATCH 1/2] replace attributes by methods; remove attribute Vinv --- .../combinatorial_polyhedron/base.pxd | 21 +- .../combinatorial_polyhedron/base.pyx | 190 ++++++++++++------ .../face_iterator.pyx | 18 +- .../polyhedron_face_lattice.pyx | 10 +- 4 files changed, 158 insertions(+), 81 deletions(-) diff --git a/src/sage/geometry/polyhedron/combinatorial_polyhedron/base.pxd b/src/sage/geometry/polyhedron/combinatorial_polyhedron/base.pxd index cda03b340ce..cbf90d36032 100644 --- a/src/sage/geometry/polyhedron/combinatorial_polyhedron/base.pxd +++ b/src/sage/geometry/polyhedron/combinatorial_polyhedron/base.pxd @@ -8,8 +8,8 @@ from .polyhedron_face_lattice cimport PolyhedronFaceLattice @cython.final cdef class CombinatorialPolyhedron(SageObject): + # Do not assume any of those attributes to be initialized, use the corresponding methods instead. cdef tuple _V # the names of VRep, if they exist - cdef dict _Vinv # dictionary to look up enumeration of vertices 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 @@ -17,10 +17,10 @@ cdef class CombinatorialPolyhedron(SageObject): cdef unsigned int _length_Vrepr # Vrepr might include rays/lines cdef size_t _n_facets # length Hrep without equalities cdef bint _unbounded # ``True`` iff Polyhedron is unbounded - cdef ListOfFaces bitrep_facets # facets in bit representation - cdef ListOfFaces bitrep_Vrepr # vertices in bit representation - cdef ListOfFaces far_face # a 'face' containing all none-vertices of Vrepr - cdef tuple far_face_tuple + cdef ListOfFaces _bitrep_facets # facets in bit representation + cdef ListOfFaces _bitrep_Vrepr # vertices in bit representation + cdef ListOfFaces _far_face # a 'face' containing all none-vertices of Vrepr + cdef tuple _far_face_tuple cdef tuple _f_vector # Edges, ridges and incidences are stored in a pointer of pointers. @@ -40,6 +40,17 @@ cdef class CombinatorialPolyhedron(SageObject): cdef size_t _n_face_lattice_incidences cdef PolyhedronFaceLattice _all_faces # class to generate Hasse diagram incidences + 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 bint unbounded(self) + cdef ListOfFaces bitrep_facets(self) + cdef ListOfFaces bitrep_Vrepr(self) + cdef ListOfFaces far_face(self) + cdef tuple far_face_tuple(self) + # Space for edges, ridges, etc. is allocated with ``MemoryAllocators``. # Upon success they are copied to ``_mem_tuple``. # Thus deallocation (at the correct time) is taken care of. diff --git a/src/sage/geometry/polyhedron/combinatorial_polyhedron/base.pyx b/src/sage/geometry/polyhedron/combinatorial_polyhedron/base.pyx index cfd0049ea01..101fe2f74c5 100644 --- a/src/sage/geometry/polyhedron/combinatorial_polyhedron/base.pyx +++ b/src/sage/geometry/polyhedron/combinatorial_polyhedron/base.pyx @@ -351,10 +351,10 @@ cdef class CombinatorialPolyhedron(SageObject): if Vrepr: # store vertices names self._V = tuple(Vrepr) - self._Vinv = {v: i for i,v in enumerate(self._V)} + Vinv = {v: i for i,v in enumerate(self._V)} else: self._V = None - self._Vinv = None + Vinv = None if facets: # store facets names and compute equalities @@ -384,18 +384,18 @@ cdef class CombinatorialPolyhedron(SageObject): self._length_Vrepr = data.nrows() # Initializing the facets in their Bit-representation. - self.bitrep_facets = incidence_matrix_to_bit_repr_of_facets(data) + self._bitrep_facets = incidence_matrix_to_bit_repr_of_facets(data) # Initializing the Vrepr as their Bit-representation. - self.bitrep_Vrepr = incidence_matrix_to_bit_repr_of_Vrepr(data) + self._bitrep_Vrepr = incidence_matrix_to_bit_repr_of_Vrepr(data) - self._n_facets = self.bitrep_facets.n_faces + self._n_facets = self.bitrep_facets().n_faces # Initialize far_face if unbounded. if self._unbounded: - 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._length_Vrepr) else: - self.far_face = None + self._far_face = None elif isinstance(data, numbers.Integral): # To construct a trivial polyhedron, equal to its affine hull, @@ -406,12 +406,12 @@ cdef class CombinatorialPolyhedron(SageObject): self._dimension = data # Initializing the facets in their Bit-representation. - self.bitrep_facets = facets_tuple_to_bit_repr_of_facets((), 0) + self._bitrep_facets = facets_tuple_to_bit_repr_of_facets((), 0) # Initializing the Vrepr as their Bit-representation. - self.bitrep_Vrepr = facets_tuple_to_bit_repr_of_Vrepr((), 0) + self._bitrep_Vrepr = facets_tuple_to_bit_repr_of_Vrepr((), 0) - self.far_face = None + self._far_face = None else: # Input is a "list" of facets. @@ -426,7 +426,7 @@ cdef class CombinatorialPolyhedron(SageObject): length_Vrepr = len(Vrepr) if Vrepr != range(len(Vrepr)): self._V = tuple(Vrepr) - self._Vinv = {v: i for i,v in enumerate(self._V)} + 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`. @@ -436,7 +436,7 @@ cdef class CombinatorialPolyhedron(SageObject): # Relabel the Vrepr to be `0,...,n`. if self._V is not None: - def f(v): return self._Vinv[v] + def f(v): return Vinv[v] else: def f(v): return int(v) facets = tuple(tuple(f(i) for i in j) for j in data) @@ -445,21 +445,21 @@ cdef class CombinatorialPolyhedron(SageObject): self._length_Hrepr = 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, length_Vrepr) # 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, length_Vrepr) # Initialize far_face if unbounded. if self._unbounded: - 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),), length_Vrepr) else: - self.far_face = None + self._far_face = None if self._unbounded: - self.far_face_tuple = tuple(far_face) + self._far_face_tuple = tuple(far_face) else: - self.far_face_tuple = () + self._far_face_tuple = () def _repr_(self): r""" @@ -546,10 +546,10 @@ cdef class CombinatorialPolyhedron(SageObject): True """ # Give a constructor by list of facets. - if self._unbounded: + if self.unbounded(): return (CombinatorialPolyhedron, (self.facets(), self.Vrepresentation(), self.Hrepresentation(), - True, self.far_face_tuple)) + True, self.far_face_tuple())) else: return (CombinatorialPolyhedron, (self.facets(), self.Vrepresentation(), self.Hrepresentation())) @@ -569,10 +569,10 @@ cdef class CombinatorialPolyhedron(SageObject): A vertex at (0, 0, 0), A ray in the direction (0, 1, 0)) """ - if self._V is not None: - return self._V + 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.length_Vrepr())) def Hrepresentation(self): r""" @@ -591,10 +591,10 @@ cdef class CombinatorialPolyhedron(SageObject): An inequality (0, 1, 1) x - 3 >= 0, An inequality (0, 0, 1) x - 1 >= 0) """ - if self._H is not None: - return self._equalities + self._H + 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.length_Hrepr())) def dimension(self): r""" @@ -613,18 +613,18 @@ cdef class CombinatorialPolyhedron(SageObject): """ if self._dimension == -2: # Dimension not computed yet. - if self._n_facets == 0: + if self.n_facets() == 0: # 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 self._unbounded or self._n_facets <= self._length_Vrepr: - self._dimension = self.bitrep_facets.compute_dimension() + self._dimension = self.length_Vrepr() - 1 + elif self.unbounded() or self.n_facets() <= self.length_Vrepr(): + self._dimension = self.bitrep_facets().compute_dimension() else: # If the polyhedron has many facets, # calculating the dimension of the dual will be faster. # The dual exists, if the polyhedron is bounded. - self._dimension = self.bitrep_facets.compute_dimension() + self._dimension = self.bitrep_facets().compute_dimension() return smallInteger(self._dimension) def n_vertices(self): @@ -670,11 +670,11 @@ cdef class CombinatorialPolyhedron(SageObject): if self.dimension() == 0: # This specific trivial polyhedron needs special attention. return smallInteger(1) - if self._unbounded: + if self.unbounded(): # 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.length_Vrepr()) def vertices(self, names=True): r""" @@ -725,11 +725,11 @@ cdef class CombinatorialPolyhedron(SageObject): """ if unlikely(self.dimension() == 0): # Handling the case of a trivial polyhedron of dimension `0`. - if names and self._V: - return (self._V[0],) + if names and self.V(): + return (self.V()[0],) else: return (smallInteger(0),) - if self._unbounded: + if self.unbounded(): it = self.face_iter(0) try: # The Polyhedron has at least one vertex. @@ -740,10 +740,10 @@ cdef class CombinatorialPolyhedron(SageObject): except StopIteration: # 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) + 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()) 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.length_Vrepr()) if not i in self.far_face_tuple()) def n_facets(self): r""" @@ -785,7 +785,7 @@ cdef class CombinatorialPolyhedron(SageObject): sage: C.n_facets() 1 """ - if unlikely(self.dimension() == 0): + if unlikely(self._dimension == 0): # This trivial polyhedron needs special attention. return smallInteger(1) return smallInteger(self._n_facets) @@ -843,7 +843,7 @@ cdef class CombinatorialPolyhedron(SageObject): # on input, so that pickle/unpickle by :meth:`reduce` works. # Every facet knows its index by the facet representation. face_iter = self.face_iter(self.dimension() - 1, dual=False) - facets = [None] * self._n_facets + facets = [None] * self.n_facets() for face in face_iter: index = face.Hrepr(names=False)[0] verts = face.Vrepr(names=names) @@ -909,9 +909,9 @@ cdef class CombinatorialPolyhedron(SageObject): cdef size_t len_edge_list = self._length_edges_list if self._edges is NULL: # compute the edges. - if self._unbounded: + if self.unbounded(): self._compute_edges(dual=False) - elif self._length_Vrepr > self._n_facets*self._n_facets: + elif self.length_Vrepr() > 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) @@ -931,8 +931,8 @@ cdef class CombinatorialPolyhedron(SageObject): # with each array containing ``len_edge_list`` of edges. # Mapping the indices of the Vrepr to the names, if requested. - if self._V is not None and names is True: - def f(size_t i): return self._V[i] + if self.V() is not None and names is True: + def f(size_t i): return self.V()[i] else: def f(size_t i): return smallInteger(i) @@ -1091,9 +1091,9 @@ cdef class CombinatorialPolyhedron(SageObject): cdef size_t len_ridge_list = self._length_edges_list if self._ridges is NULL: # compute the ridges. - if self._unbounded: + if self.unbounded(): self._compute_ridges(dual=False) - elif self._length_Vrepr*self._length_Vrepr < self._n_facets: + elif self.length_Vrepr()*self.length_Vrepr() < self.n_facets(): # This is a wild estimate # that in this case it is better to use the dual. self._compute_edges(dual=True) @@ -1114,8 +1114,8 @@ cdef class CombinatorialPolyhedron(SageObject): # with each array containing ``len_ridge_list`` of ridges. # Mapping the indices of the Vepr to the names, if requested. - if self._H is not None and names is True: - def f(size_t i): return self._H[i] + if self.H() is not None and names is True: + def f(size_t i): return self.H()[i] else: def f(size_t i): return smallInteger(i) @@ -1129,8 +1129,8 @@ cdef class CombinatorialPolyhedron(SageObject): if add_equalities and names: # Also getting the equalities for each facet. return tuple( - (((facet_one(i),) + self._equalities), - ((facet_two(i),) + self._equalities)) + (((facet_one(i),) + self._equalities()), + ((facet_two(i),) + self._equalities())) for i in range(n_ridges)) else: return tuple((facet_one(i), facet_two(i)) @@ -1326,7 +1326,7 @@ cdef class CombinatorialPolyhedron(SageObject): cdef FaceIterator face_iter if dual is None: # Determine the faster way, to iterate through all faces. - if self._unbounded or self._n_facets <= self._length_Vrepr: + if self.unbounded() or self.n_facets() <= self.length_Vrepr(): dual = False else: dual = True @@ -1343,7 +1343,7 @@ cdef class CombinatorialPolyhedron(SageObject): See :meth:`CombinatorialPolyhedron.face_iter` """ - if dual and self._unbounded: + if dual and self.unbounded(): raise ValueError("cannot iterate over dual of unbounded polyhedron") if dimension == -2: return FaceIterator(self, dual) @@ -1538,6 +1538,72 @@ cdef class CombinatorialPolyhedron(SageObject): # Let ``_all_faces`` determine Vrepresentation. return self._all_faces.get_face(dim, newindex) + cdef tuple V(self): + r""" + Return the names of the Vrepresentation, if they exist. Else return ``None``. + """ + return self._V + + cdef tuple H(self): + r""" + Return the names Hrepresentatives, which are facets. + + If not given, return ``None``. + """ + return self._H + + cdef tuple equalities(self): + r""" + Return the names of the equalities. + + If not equalities are given, return ``None``. + """ + return self._equalities + + cdef unsigned int length_Vrepr(self): + r""" + Return the number of elements in the Vrepresentation. + """ + return self._length_Vrepr + + cdef unsigned int length_Hrepr(self): + r""" + Return the number of elements in the Hrepresentation. + """ + return self._length_Hrepr + + cdef bint unbounded(self): + r""" + Return whether the polyhedron is unbounded. + """ + return self._unbounded + + cdef ListOfFaces bitrep_facets(self): + r""" + Return the facets in bit representation. + """ + return self._bitrep_facets + + cdef ListOfFaces bitrep_Vrepr(self): + r""" + Return the Vrepresentations in bit representation. + """ + return self._bitrep_Vrepr + + cdef ListOfFaces far_face(self): + r""" + Return a list with only the far face. + + This is a face containing all Vrepresentatives that are not vertices. + """ + return self._far_face + + cdef tuple far_face_tuple(self): + r""" + Return the far face as it was given on initialization. + """ + return self._far_face_tuple + cdef int _compute_f_vector(self) except -1: r""" Compute the ``f_vector`` of the polyhedron. @@ -1548,7 +1614,7 @@ cdef class CombinatorialPolyhedron(SageObject): return 0 # There is no need to recompute the f_vector. cdef bint dual - if self._unbounded or self._n_facets <= self._length_Vrepr: + if self.unbounded() or self.n_facets() <= self.length_Vrepr(): # In this case the non-dual approach is faster.. dual = False else: @@ -1567,7 +1633,7 @@ cdef class CombinatorialPolyhedron(SageObject): # For each face in the iterator, add `1` to the corresponding entry in # ``f_vector``. - if self._n_facets > 0 and dim > 0: + if self.n_facets() > 0 and dim > 0: d = face_iter.next_dimension() while (d < dim): sig_check() @@ -1576,7 +1642,7 @@ cdef class CombinatorialPolyhedron(SageObject): # Copy ``f_vector``. if dual: - if dim > 1 and f_vector[1] < self._n_facets: + if dim > 1 and f_vector[1] < self.n_facets(): # The input seemed to be wrong. raise ValueError("not all facets are joins of vertices") @@ -1586,8 +1652,8 @@ cdef class CombinatorialPolyhedron(SageObject): tuple(smallInteger(f_vector[dim+1-i]) for i in range(dim+2)) else: - if not self._unbounded and dim > 1 \ - and f_vector[1] < self._length_Vrepr - len(self.far_face_tuple): + if not self.unbounded() and dim > 1 \ + and f_vector[1] < self.length_Vrepr() - len(self.far_face_tuple()): # The input seemed to be wrong. raise ValueError("not all vertices are intersections of facets") @@ -1682,7 +1748,7 @@ cdef class CombinatorialPolyhedron(SageObject): # requires the dimension of the original polyhedron face_iter = self._face_iter(dual, dim - 2) - if self._n_facets > 0 and dim > 0: + if self.n_facets() > 0 and dim > 0: # If not, there won't even be any edges. Prevent error message. while (face_iter.next_dimension() == 1): @@ -1729,7 +1795,7 @@ cdef class CombinatorialPolyhedron(SageObject): f_vector[dim + 1] = 1 # This is not a proper face. counter = 0 - if self._n_facets > 0 and dim > 0: + if self.n_facets() > 0 and dim > 0: # If not, there won't even be any edges. Prevent error message. d = face_iter.next_dimension() @@ -1804,7 +1870,7 @@ cdef class CombinatorialPolyhedron(SageObject): cdef size_t counter = 0 # the number of ridges so far cdef size_t current_length = 1 # dynamically enlarge **ridges - if dim == 1 and self._n_facets > 1: + if dim == 1 and self.n_facets() > 1: # In this case there is a ridge, but its not a proper face. ridges[0] = mem.allocarray(2, sizeof(size_t)) ridges[0][0] = 0 @@ -1853,7 +1919,7 @@ cdef class CombinatorialPolyhedron(SageObject): else: face_iter = self._face_iter(dual, dim -2) - if self._n_facets > 1 and dim > 0: + if self.n_facets() > 1 and dim > 0: # If not, there won't even be any ridges # as intersection of two distinct facets. # Prevent error message. diff --git a/src/sage/geometry/polyhedron/combinatorial_polyhedron/face_iterator.pyx b/src/sage/geometry/polyhedron/combinatorial_polyhedron/face_iterator.pyx index 339747d5c64..74cbedbf0ac 100644 --- a/src/sage/geometry/polyhedron/combinatorial_polyhedron/face_iterator.pyx +++ b/src/sage/geometry/polyhedron/combinatorial_polyhedron/face_iterator.pyx @@ -440,15 +440,15 @@ cdef class FaceIterator(SageObject): self.output_dimension = -2 if dual: - self.atoms = C.bitrep_facets - self.coatoms = C.bitrep_Vrepr + self.atoms = C.bitrep_facets() + self.coatoms = C.bitrep_Vrepr() else: - self.coatoms = C.bitrep_facets - self.atoms = C.bitrep_Vrepr + self.coatoms = C.bitrep_facets() + self.atoms = C.bitrep_Vrepr() self.face_length = self.coatoms.face_length - self._V = C._V - self._H = C._H - self._equalities = C._equalities + self._V = C.V() + self._H = C.H() + self._equalities = C.equalities() self.atom_repr = self._mem.allocarray(self.coatoms.n_atoms, sizeof(size_t)) self.coatom_repr = self._mem.allocarray(self.coatoms.n_faces, sizeof(size_t)) @@ -475,7 +475,7 @@ cdef class FaceIterator(SageObject): self.visited_all = self._mem.allocarray(self.coatoms.n_faces, sizeof(uint64_t *)) self.n_visited_all = self._mem.allocarray(self.dimension, sizeof(size_t)) self.n_visited_all[self.dimension -1] = 0 - if C._unbounded: + if C.unbounded(): # Treating the far face as if we had visited all its elements. # Hence we will visit all intersections of facets unless contained in the far face. @@ -484,7 +484,7 @@ cdef class FaceIterator(SageObject): # needs to be at most ``n_facets - 1``. # Hence it is fine to use the first entry already for the far face, # as ``self.visited_all`` holds ``n_facets`` pointers. - some_list = C.far_face + some_list = C.far_face() self.visited_all[0] = some_list.data[0] self.n_visited_all[self.dimension -1] = 1 diff --git a/src/sage/geometry/polyhedron/combinatorial_polyhedron/polyhedron_face_lattice.pyx b/src/sage/geometry/polyhedron/combinatorial_polyhedron/polyhedron_face_lattice.pyx index 3a67d1fc5d5..a5b282a9fbc 100644 --- a/src/sage/geometry/polyhedron/combinatorial_polyhedron/polyhedron_face_lattice.pyx +++ b/src/sage/geometry/polyhedron/combinatorial_polyhedron/polyhedron_face_lattice.pyx @@ -128,15 +128,15 @@ cdef class PolyhedronFaceLattice: self._mem = MemoryAllocator() self.dimension = C.dimension() self.dual = False - if C.bitrep_facets.n_faces > C.bitrep_Vrepr.n_faces: + if C.bitrep_facets().n_faces > C.bitrep_Vrepr().n_faces: self.dual = True - if C._unbounded: + if C.unbounded(): self.dual = False cdef FaceIterator face_iter = C._face_iter(self.dual, -2) self.face_length = face_iter.face_length - self._V = C._V - self._H = C._H - self._equalities = C._equalities + self._V = C.V() + self._H = C.H() + self._equalities = C.equalities() # copy f_vector for later use f_vector = C.f_vector() From d4b3163762932706ba2af5493e55c4d4c4a58ed0 Mon Sep 17 00:00:00 2001 From: Jonathan Kliem Date: Wed, 13 Nov 2019 13:43:08 +0100 Subject: [PATCH 2/2] small fix in ridges --- .../geometry/polyhedron/combinatorial_polyhedron/base.pyx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sage/geometry/polyhedron/combinatorial_polyhedron/base.pyx b/src/sage/geometry/polyhedron/combinatorial_polyhedron/base.pyx index 101fe2f74c5..f773a675a95 100644 --- a/src/sage/geometry/polyhedron/combinatorial_polyhedron/base.pyx +++ b/src/sage/geometry/polyhedron/combinatorial_polyhedron/base.pyx @@ -1129,8 +1129,8 @@ cdef class CombinatorialPolyhedron(SageObject): if add_equalities and names: # Also getting the equalities for each facet. return tuple( - (((facet_one(i),) + self._equalities()), - ((facet_two(i),) + self._equalities())) + (((facet_one(i),) + self.equalities()), + ((facet_two(i),) + self.equalities())) for i in range(n_ridges)) else: return tuple((facet_one(i), facet_two(i))