Skip to content

Commit

Permalink
Trac #32666: Polyhedron_base.vertex_adjacency_matrix: Do not use face…
Browse files Browse the repository at this point in the history
…_lattice

instead just go through `self.combinatorial_polyhedron().edges()`

This should be faster - and only avoids an indirect dependency of
`sage.geometry.polyhedron.plot` on `sage.combinat`

URL: https://trac.sagemath.org/32666
Reported by: mkoeppe
Ticket author(s): Jonathan Kliem
Reviewer(s): Frédéric Chapoton, Matthias Koeppe
  • Loading branch information
Release Manager committed Oct 19, 2021
2 parents de6b6a1 + 2cd253c commit ba408ef
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 62 deletions.
35 changes: 1 addition & 34 deletions src/sage/geometry/polyhedron/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -471,39 +471,6 @@ def set_adjacent(h1, h2):
M.set_immutable()
return M

def _vertex_adjacency_matrix(self):
"""
Compute the vertex adjacency matrix in case it has not been
computed during initialization.
EXAMPLES::
sage: p = Polyhedron(vertices=[(0,0),(1,0),(0,1)])
sage: p._vertex_adjacency_matrix()
[0 1 1]
[1 0 1]
[1 1 0]
"""
# TODO: This implementation computes the whole face lattice,
# which is much more information than necessary.
M = matrix(ZZ, self.n_Vrepresentation(), self.n_Vrepresentation(), 0)

def set_adjacent(v1, v2):
if v1 is v2:
return
i = v1.index()
j = v2.index()
M[i, j] = 1
M[j, i] = 1

face_lattice = self.face_lattice()
for face in face_lattice:
Vrep = face.ambient_Vrepresentation()
if len(Vrep) == 2:
set_adjacent(Vrep[0], Vrep[1])
M.set_immutable()
return M

def _delete(self):
"""
Delete this polyhedron.
Expand Down Expand Up @@ -2786,7 +2753,7 @@ def vertex_adjacency_matrix(self):
sage: P.adjacency_matrix().is_immutable()
True
"""
return self._vertex_adjacency_matrix()
return self.combinatorial_polyhedron().vertex_adjacency_matrix()

adjacency_matrix = vertex_adjacency_matrix

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ cdef class CombinatorialPolyhedron(SageObject):
cdef inline int _compute_ridges(self, dual) except -1:
return self._compute_edges_or_ridges(dual, False)

cdef int _compute_edges_or_ridges(self, bint dual, bint do_edges) except -1
cdef int _compute_edges_or_ridges(self, int dual, bint do_edges) except -1
cdef size_t _compute_edges_or_ridges_with_iterator(
self, FaceIterator face_iter, const bint do_atom_rep, const bint do_f_vector,
size_t ***edges_pt, size_t *counter_pt, size_t *current_length_pt,
Expand Down
105 changes: 78 additions & 27 deletions src/sage/geometry/polyhedron/combinatorial_polyhedron/base.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -1185,19 +1185,7 @@ cdef class CombinatorialPolyhedron(SageObject):
('a', 'c'),
('a', 'b'))
"""
if self._edges is NULL:
# compute the edges.
if not self.is_bounded():
self._compute_edges(dual=False)
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)
else:
# In most bounded cases, one should use the dual.
self._compute_edges(dual=True)
if self._edges is NULL:
raise ValueError('could not determine edges')
self._compute_edges(-1)

# Mapping the indices of the Vrep to the names, if requested.
if self.Vrep() is not None and names is True:
Expand Down Expand Up @@ -1247,6 +1235,53 @@ cdef class CombinatorialPolyhedron(SageObject):

graph = vertex_graph

@cached_method
def vertex_adjacency_matrix(self):
"""
Return the binary matrix of vertex adjacencies.
.. SEEALSO::
:meth:`~sage.geometry.polyhedron.base.Polyhedron_base.vertex_adjacency_matrix`.
EXAMPLES::
sage: P = polytopes.cube()
sage: C = P.combinatorial_polyhedron()
sage: C.vertex_adjacency_matrix()
[0 1 0 1 0 1 0 0]
[1 0 1 0 0 0 1 0]
[0 1 0 1 0 0 0 1]
[1 0 1 0 1 0 0 0]
[0 0 0 1 0 1 0 1]
[1 0 0 0 1 0 1 0]
[0 1 0 0 0 1 0 1]
[0 0 1 0 1 0 1 0]
TESTS::
sage: CombinatorialPolyhedron(-1).vertex_adjacency_matrix()
[]
sage: CombinatorialPolyhedron(0).vertex_adjacency_matrix()
[0]
sage: polytopes.cube().vertex_adjacency_matrix().is_immutable()
True
"""
from sage.rings.integer_ring import ZZ
from sage.matrix.constructor import matrix
cdef Matrix_integer_dense adjacency_matrix = matrix(
ZZ, self.n_Vrepresentation(), self.n_Vrepresentation(), 0)
cdef size_t i, a, b

self._compute_edges(-1)
for i in range(self._n_edges):
a = self._get_edge(self._edges, i, 0)
b = self._get_edge(self._edges, i, 1)
adjacency_matrix.set_unsafe_si(a, b, 1)
adjacency_matrix.set_unsafe_si(b, a, 1)
adjacency_matrix.set_immutable()
return adjacency_matrix

def edge_graph(self, names=True):
r"""
Return the edge graph.
Expand Down Expand Up @@ -1371,19 +1406,7 @@ cdef class CombinatorialPolyhedron(SageObject):
from sage.misc.superseded import deprecation
deprecation(31834, "the keyword ``add_equalities`` is deprecated; use ``add_equations``", 3)
add_equations = True
if self._ridges is NULL:
# compute the ridges.
if not self.is_bounded():
self._compute_ridges(dual=False)
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_ridges(dual=True)
else:
# In most bounded cases, one should not use the dual.
self._compute_ridges(dual=False)
if self._ridges is NULL:
raise ValueError('could not determine ridges')
self._compute_ridges(-1)
n_ridges = self._n_ridges

# Mapping the indices of the Vepr to the names, if requested.
Expand Down Expand Up @@ -3209,11 +3232,12 @@ cdef class CombinatorialPolyhedron(SageObject):

self._f_vector = tuple(smallInteger(f_vector[i]) for i in range(dim+2))

cdef int _compute_edges_or_ridges(self, bint dual, bint do_edges) except -1:
cdef int _compute_edges_or_ridges(self, int dual, bint do_edges) except -1:
r"""
Compute the edges of the polyhedron if ``edges`` else the ridges.
If ``dual``, use the face iterator in dual mode, else in non-dual.
If ``dual`` is ``-1`` determine this automatically.
If the ``f_vector`` is unkown computes it as well if computing the edges
in non-dual mode or the ridges in dual-mode.
Expand All @@ -3223,6 +3247,27 @@ cdef class CombinatorialPolyhedron(SageObject):
if (self._edges is not NULL and do_edges) or (self._ridges is not NULL and not do_edges):
return 0 # There is no need to recompute.

if dual == -1:
# Determine whether to use dual mode or not.
if not self.is_bounded():
dual = 0
elif do_edges:
if 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.
dual = 0
else:
# In most bounded cases, one should use the dual.
dual = 1
else:
if 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.
dual = 1
else:
# In most bounded cases, one should not use the dual.
dual = 0

cdef MemoryAllocator mem = MemoryAllocator()
cdef FaceIterator face_iter
cdef int dim = self.dimension()
Expand Down Expand Up @@ -3297,6 +3342,12 @@ cdef class CombinatorialPolyhedron(SageObject):
self._mem_tuple += (mem,)
sig_unblock()

if do_edges and self._edges is NULL:
raise ValueError('could not determine edges')
elif not do_edges and self._ridges is NULL:
raise ValueError('could not determine ridges')


cdef size_t _compute_edges_or_ridges_with_iterator(
self, FaceIterator face_iter, const bint do_atom_rep, const bint do_f_vector,
size_t ***edges_pt, size_t *counter_pt, size_t *current_length_pt,
Expand Down

0 comments on commit ba408ef

Please sign in to comment.