Trac #28894: Move most important attributes of FaceIterator to a stru…

…cture.

This ticket is part of a series of tickets to parallelize the f-vector
for polyhedra.

Python classes may not be accessed in `nogil`. To access important
attributes of the class `FaceIterator`, we move those attributes to a
structure, which can be accessed in `nogil`.

Along the way, the ticket simplifies some doctests that define cython
functions.

URL: https://trac.sagemath.org/28894
Reported by: gh-kliem
Ticket author(s): Jonathan Kliem
Reviewer(s): Jean-Philippe Labbé, Frédéric Chapoton

src/sage/geometry/polyhedron/combinatorial_polyhedron/base.pyx

@@ -1883,7 +1883,7 @@ cdef class CombinatorialPolyhedron(SageObject):
         cdef simpliciality = dim - 1
 
         # For each face in the iterator, check if its a simplex.
-        face_iter.lowest_dimension = 2 # every 1-face is a simplex
+        face_iter.structure.lowest_dimension = 2 # every 1-face is a simplex
         d = face_iter.next_dimension()
         while (d < dim):
             sig_check()
@@ -1997,7 +1997,7 @@ cdef class CombinatorialPolyhedron(SageObject):
         cdef simplicity = dim - 1
 
         # For each coface in the iterator, check if its a simplex.
-        coface_iter.lowest_dimension = 2 # every coface of dimension 1 is a simplex
+        coface_iter.structure.lowest_dimension = 2 # every coface of dimension 1 is a simplex
         d = coface_iter.next_dimension()
         while (d < dim):
             sig_check()
@@ -2892,8 +2892,8 @@ cdef class CombinatorialPolyhedron(SageObject):
                     face_iter.set_atom_rep()
 
                     # Copy the information.
-                    edges[one][2*two] = face_iter.atom_rep[0]
-                    edges[one][2*two + 1] = face_iter.atom_rep[1]
+                    edges[one][2*two] = face_iter.structure.atom_rep[0]
+                    edges[one][2*two + 1] = face_iter.structure.atom_rep[1]
                     counter += 1
 
             # Success, copy the data to ``CombinatorialPolyhedron``.
@@ -2944,8 +2944,8 @@ cdef class CombinatorialPolyhedron(SageObject):
                         face_iter.set_atom_rep()
 
                         # Copy the information.
-                        edges[one][2*two] = face_iter.atom_rep[0]
-                        edges[one][2*two + 1] = face_iter.atom_rep[1]
+                        edges[one][2*two] = face_iter.structure.atom_rep[0]
+                        edges[one][2*two + 1] = face_iter.structure.atom_rep[1]
                         counter += 1
 
                     d = face_iter.next_dimension()  # Go to next face.
@@ -3065,8 +3065,8 @@ cdef class CombinatorialPolyhedron(SageObject):
                 face_iter.set_coatom_rep()
 
                 # Copy the information.
-                ridges[one][2*two] = face_iter.coatom_rep[0]
-                ridges[one][2*two + 1] = face_iter.coatom_rep[1]
+                ridges[one][2*two] = face_iter.structure.coatom_rep[0]
+                ridges[one][2*two + 1] = face_iter.structure.coatom_rep[1]
                 counter += 1
 
         # Success, copy the data to ``CombinatorialPolyhedron``.

src/sage/geometry/polyhedron/combinatorial_polyhedron/combinatorial_face.pyx

@@ -183,19 +183,19 @@ cdef class CombinatorialFace(SageObject):
             # Copy data from FaceIterator.
             it = data
             self._dual              = it.dual
-            self.face_mem           = ListOfFaces(1, it.face_length*64)
+            self.face_mem           = ListOfFaces(1, it.structure.face_length*64)
             self.face               = self.face_mem.data[0]
-            memcpy(self.face, it.face, it.face_length*8)
+            memcpy(self.face, it.structure.face, it.structure.face_length*8)
             self._mem               = MemoryAllocator()
-            self._dimension         = it.current_dimension
-            self._ambient_dimension = it.dimension
-            self.face_length        = it.face_length
+            self._dimension         = it.structure.current_dimension
+            self._ambient_dimension = it.structure.dimension
+            self.face_length        = it.structure.face_length
             self._ambient_Vrep      = it._Vrep
             self._ambient_facets    = it._facet_names
             self._equalities        = it._equalities
             self.atoms              = it.atoms
             self.coatoms            = it.coatoms
-            self._hash_index        = it._index
+            self._hash_index        = it.structure._index
 
         elif isinstance(data, PolyhedronFaceLattice):
             all_faces = data

src/sage/geometry/polyhedron/combinatorial_polyhedron/conversions.pyx

@@ -254,10 +254,9 @@ def incidence_matrix_to_bit_rep_of_facets(matrix):
         ....: from sage.ext.memory_allocator cimport MemoryAllocator
         ....: from libc.stdint cimport uint64_t
         ....:
-        ....: def bit_rep_to_Vrep_list_wrapper(list_of_faces, index):
+        ....: def bit_rep_to_Vrep_list_wrapper(ListOfFaces faces, index):
         ....:     cdef MemoryAllocator mem = MemoryAllocator()
         ....:     cdef size_t *output
-        ....:     cdef ListOfFaces faces = list_of_faces
         ....:     output = <size_t *> mem.allocarray(faces.n_atoms,
         ....:                                        sizeof(size_t))
         ....:     cdef uint64_t * data = faces.data[index]
@@ -343,10 +342,9 @@ def incidence_matrix_to_bit_rep_of_Vrep(matrix):
         ....: from sage.ext.memory_allocator cimport MemoryAllocator
         ....: from libc.stdint cimport uint64_t
         ....:
-        ....: def bit_rep_to_Vrep_list_wrapper(list_of_faces, index):
+        ....: def bit_rep_to_Vrep_list_wrapper(ListOfFaces faces, index):
         ....:     cdef MemoryAllocator mem = MemoryAllocator()
         ....:     cdef size_t *output
-        ....:     cdef ListOfFaces faces = list_of_faces
         ....:     output = <size_t *> mem.allocarray(faces.n_atoms,
         ....:                                        sizeof(size_t))
         ....:     cdef uint64_t * data = faces.data[index]
@@ -439,10 +437,9 @@ def facets_tuple_to_bit_rep_of_facets(tuple facets_input, size_t n_Vrep):
         ....: from sage.ext.memory_allocator cimport MemoryAllocator
         ....: from libc.stdint cimport uint64_t
         ....:
-        ....: def bit_rep_to_Vrep_list_wrapper(list_of_faces, index):
+        ....: def bit_rep_to_Vrep_list_wrapper(ListOfFaces faces, index):
         ....:     cdef MemoryAllocator mem = MemoryAllocator()
         ....:     cdef size_t *output
-        ....:     cdef ListOfFaces faces = list_of_faces
         ....:     output = <size_t *> mem.allocarray(faces.n_atoms,
         ....:                                        sizeof(size_t))
         ....:     cdef uint64_t * data = faces.data[index]
@@ -506,10 +503,9 @@ def facets_tuple_to_bit_rep_of_Vrep(tuple facets_input, size_t n_Vrep):
         ....: from sage.ext.memory_allocator cimport MemoryAllocator
         ....: from libc.stdint cimport uint64_t
         ....:
-        ....: def bit_rep_to_Vrep_list_wrapper(list_of_faces, index):
+        ....: def bit_rep_to_Vrep_list_wrapper(ListOfFaces faces, index):
         ....:     cdef MemoryAllocator mem = MemoryAllocator()
         ....:     cdef size_t *output
-        ....:     cdef ListOfFaces faces = list_of_faces
         ....:     output = <size_t *> mem.allocarray(faces.n_atoms,
         ....:                                        sizeof(size_t))
         ....:     cdef uint64_t * data = faces.data[index]

src/sage/geometry/polyhedron/combinatorial_polyhedron/face_iterator.pxd

@@ -5,27 +5,17 @@ from sage.structure.sage_object cimport SageObject
 from .list_of_faces             cimport ListOfFaces
 from .combinatorial_face        cimport CombinatorialFace
 
-@cython.final
-cdef class FaceIterator(SageObject):
-    cdef readonly bint dual         # if 1, then iterate over dual Polyhedron
-    cdef uint64_t *face             # the current face of the iterator
-    cdef size_t *atom_rep           # a place where atom-representation of face will be stored
-    cdef size_t *coatom_rep         # a place where coatom-representation of face will be stored
-    cdef int current_dimension      # dimension of current face, dual dimension if ``dual``
-    cdef int dimension              # dimension of the polyhedron
-    cdef int output_dimension       # only faces of this (dual?) dimension are considered
-    cdef int lowest_dimension       # don't consider faces below this (dual?) dimension
-    cdef size_t _index              # this counts the number of seen faces, useful for hasing the faces
-    cdef MemoryAllocator _mem
-    cdef tuple newfaces_lists       # tuple to hold the ListOfFaces corresponding to maybe_newfaces
-    cdef size_t face_length         # stores length of the faces in terms of uint64_t
-
-    # some copies from ``CombinatorialPolyhedron``
-    cdef tuple _Vrep, _facet_names, _equalities
-
-    # Atoms and coatoms are the vertices/facets of the Polyedron.
-    # If ``dual == 0``, then coatoms are facets, atoms vertices and vice versa.
-    cdef ListOfFaces atoms, coatoms
+cdef struct iter_struct:
+    bint dual                  # if 1, then iterate over dual Polyhedron
+    uint64_t *face             # the current face of the iterator
+    size_t *atom_rep           # a place where atom-representaion of face will be stored
+    size_t *coatom_rep         # a place where coatom-representaion of face will be stored
+    int current_dimension      # dimension of current face, dual dimension if ``dual``
+    int dimension              # dimension of the polyhedron
+    int output_dimension       # only faces of this (dual?) dimension are considered
+    int lowest_dimension       # don't consider faces below this (dual?) dimension
+    size_t _index              # this counts the number of seen faces, useful for hasing the faces
+    size_t face_length         # stores length of the faces in terms of uint64_t
 
     # ``visited_all`` points to faces, of which we have visited all faces already.
     # The number of faces in ``visited_all` might depend on the current dimension:
@@ -37,30 +27,45 @@ cdef class FaceIterator(SageObject):
     #     In this way, we will append ``visited_all`` in lower dimension, but
     #     will ignore those changes when going up in dimension again.
     #     This is why the number of faces in ``visited_all``depends on dimension.
-    cdef uint64_t **visited_all
-    cdef size_t *n_visited_all
+    uint64_t **visited_all
+    size_t *n_visited_all
 
     # ``maybe_newfaces`` is where all possible facets of a face are stored.
     # In dimension ``dim`` when visiting all faces of some face,
     # the intersections with other faces are stored in ``newfaces2[dim]``.
-    cdef uint64_t ***maybe_newfaces
+    uint64_t ***maybe_newfaces
 
     # ``newfaces`` will point to those faces in ``maybe_newfaces``
     # that are of codimension 1 and not already visited.
-    cdef uint64_t ***newfaces
-    cdef size_t *n_newfaces  # number of newfaces for each dimension
+    uint64_t ***newfaces
+    size_t *n_newfaces  # number of newfaces for each dimension
 
     # After having visited a face completely, we want to add it to ``visited_all``.
     # ``first_dim[i]`` will indicate, wether there is one more face in
     # ``newfaces[i]`` then ``n_newfaces[i]`` suggests
     # that has to be added to ``visited_all``.
     # If ``first_time[i] == False``, we still need to
     # add ``newfaces[i][n_newfaces[i]]`` to ``visited_all``.
-    cdef bint *first_time
+    bint *first_time
 
     # The number of elements in newfaces[current_dimension],
     # that have not been visited yet.
-    cdef size_t yet_to_visit
+    size_t yet_to_visit
+
+
+@cython.final
+cdef class FaceIterator(SageObject):
+    cdef iter_struct structure
+    cdef readonly bint dual         # if 1, then iterate over dual Polyhedron
+    cdef MemoryAllocator _mem
+    cdef tuple newfaces_lists       # tuple to hold the ListOfFaces corresponding to maybe_newfaces
+
+    # some copies from ``CombinatorialPolyhedron``
+    cdef tuple _Vrep, _facet_names, _equalities
+
+    # Atoms and coatoms are the vertices/facets of the Polyedron.
+    # If ``dual == 0``, then coatoms are facets, atoms vertices and vice versa.
+    cdef ListOfFaces atoms, coatoms
 
     cdef inline CombinatorialFace next_face(self)
     cdef inline int next_dimension(self) except -1