Skip to content

Commit

Permalink
gh-36623: cylint cleanup in combinatorial polyhedra
Browse files Browse the repository at this point in the history
    
fixes all cython-lint warnings in combinatorial_polyhedron folder

### 📝 Checklist

- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
    
URL: #36623
Reported by: Frédéric Chapoton
Reviewer(s): Kwankyu Lee
  • Loading branch information
Release Manager committed Dec 4, 2023
2 parents 7b3e208 + 6f8168f commit 69d03d3
Show file tree
Hide file tree
Showing 13 changed files with 90 additions and 91 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ from sage.geometry.polyhedron.combinatorial_polyhedron.list_of_faces
from sage.geometry.polyhedron.combinatorial_polyhedron.face_data_structure cimport face_t
from sage.geometry.polyhedron.combinatorial_polyhedron.polyhedron_face_lattice cimport PolyhedronFaceLattice


@cython.final
cdef class CombinatorialPolyhedron(SageObject):
cdef public dict _cached_methods
Expand Down
35 changes: 14 additions & 21 deletions src/sage/geometry/polyhedron/combinatorial_polyhedron/base.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,10 @@ from sage.geometry.cone import ConvexRationalPolyhedralCone
from sage.structure.element import Matrix
from sage.matrix.matrix_dense cimport Matrix_dense
from sage.misc.misc import is_iterator
from .conversions \
import incidence_matrix_to_bit_rep_of_facets, \
incidence_matrix_to_bit_rep_of_Vrep, \
facets_tuple_to_bit_rep_of_facets, \
facets_tuple_to_bit_rep_of_Vrep
from .conversions import (incidence_matrix_to_bit_rep_of_facets,
incidence_matrix_to_bit_rep_of_Vrep,
facets_tuple_to_bit_rep_of_facets,
facets_tuple_to_bit_rep_of_Vrep)
from sage.geometry.polyhedron.combinatorial_polyhedron.conversions cimport Vrep_list_to_bit_rep
from sage.misc.cachefunc import cached_method

Expand Down Expand Up @@ -437,7 +436,7 @@ cdef class CombinatorialPolyhedron(SageObject):
from sage.matrix.constructor import matrix
from sage.rings.integer_ring import ZZ
incidence_matrix = matrix(ZZ, data.incidence_matrix().rows()
+ [[ZZ.one() for _ in range(len(data.facet_normals()))]])
+ [[ZZ.one() for _ in range(len(data.facet_normals()))]])
return self._init_from_incidence_matrix(incidence_matrix)

cdef _init_facet_names(self, facets) noexcept:
Expand Down Expand Up @@ -481,11 +480,10 @@ cdef class CombinatorialPolyhedron(SageObject):
data.set_immutable()
self.incidence_matrix.set_cache(data)


# Delete equations.
data = data.delete_columns(
[i for i in range(data.ncols())
if all(data[j,i] for j in range(data.nrows()))],
if all(data[j, i] for j in range(data.nrows()))],
check=False)

# Initializing the facets in their Bit-representation.
Expand Down Expand Up @@ -515,7 +513,7 @@ cdef class CombinatorialPolyhedron(SageObject):
n_Vrepresentation = len(Vrep)
if Vrep != range(len(Vrep)):
self._Vrep = tuple(Vrep)
Vinv = {v: i for i,v in enumerate(self._Vrep)}
Vinv = {v: i for i, v in enumerate(self._Vrep)}
else:
# Assuming the user gave as correct names for the vertices
# and labeled them instead by `0,...,n`.
Expand Down Expand Up @@ -548,7 +546,7 @@ cdef class CombinatorialPolyhedron(SageObject):
Initialize self from two ``ListOfFaces``.
"""
self._bitrep_facets = facets
self._bitrep_Vrep = Vrep
self._bitrep_Vrep = Vrep

self._n_Hrepresentation = self._bitrep_facets.n_faces()
self._n_Vrepresentation = self._bitrep_Vrep.n_faces()
Expand Down Expand Up @@ -629,7 +627,7 @@ cdef class CombinatorialPolyhedron(SageObject):
'A 2-dimensional combinatorial polyhedron with 1 facet'
"""
desc = "A {}-dimensional combinatorial polyhedron with {} facet"\
.format(self.dimension(), self.n_facets())
.format(self.dimension(), self.n_facets())
if self.n_facets() != 1:
desc += "s"
return desc
Expand Down Expand Up @@ -1694,7 +1692,7 @@ cdef class CombinatorialPolyhedron(SageObject):

if not names:
vertices = [i for i in range(n_facets + n_Vrep)]
edges = tuple((j, n_Vrep + n_facets - 1 - i) for i,facet in enumerate(facet_iter) for j in facet.ambient_V_indices())
edges = tuple((j, n_Vrep + n_facets - 1 - i) for i, facet in enumerate(facet_iter) for j in facet.ambient_V_indices())
else:
facet_names = self.facet_names()
if facet_names is None:
Expand All @@ -1707,7 +1705,7 @@ cdef class CombinatorialPolyhedron(SageObject):
Vrep = [("V", i) for i in range(n_Vrep)]

vertices = Vrep + facet_names
edges = tuple((Vrep[j], facet_names[n_facets - 1 - i]) for i,facet in enumerate(facet_iter) for j in facet.ambient_V_indices())
edges = tuple((Vrep[j], facet_names[n_facets - 1 - i]) for i, facet in enumerate(facet_iter) for j in facet.ambient_V_indices())
return DiGraph([vertices, edges], format='vertices_and_edges', immutable=True)

@cached_method
Expand Down Expand Up @@ -2126,7 +2124,7 @@ cdef class CombinatorialPolyhedron(SageObject):
cdef simpliciality = dim - 1

# For each face in the iterator, check if its a simplex.
face_iter.structure.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()
Expand Down Expand Up @@ -2237,7 +2235,7 @@ cdef class CombinatorialPolyhedron(SageObject):
cdef simplicity = dim - 1

# For each coface in the iterator, check if its a simplex.
coface_iter.structure.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()
Expand Down Expand Up @@ -2623,7 +2621,6 @@ cdef class CombinatorialPolyhedron(SageObject):

return (False, None)


def join_of_Vrep(self, *indices):
r"""
Return the smallest face containing all Vrepresentatives indicated by the indices.
Expand Down Expand Up @@ -2793,7 +2790,6 @@ cdef class CombinatorialPolyhedron(SageObject):
if 'dual' in kwds and dual == -1 and kwds['dual'] in (False, True):
dual = int(kwds['dual'])

cdef FaceIterator face_iter
if dual == -1:
# Determine the faster way, to iterate through all faces.
if not self.is_bounded() or self.n_facets() <= self.n_Vrepresentation():
Expand Down Expand Up @@ -3354,7 +3350,6 @@ cdef class CombinatorialPolyhedron(SageObject):
and self.far_face_tuple() == other_C.far_face_tuple()
and self.incidence_matrix() == other.incidence_matrix())


# Methods to obtain a different combinatorial polyhedron.

cpdef CombinatorialPolyhedron dual(self) noexcept:
Expand Down Expand Up @@ -3484,7 +3479,6 @@ cdef class CombinatorialPolyhedron(SageObject):

return CombinatorialPolyhedron((new_facets, new_Vrep), Vrep=new_Vrep_names, facets=new_facet_names)


# Internal methods.

cdef int _compute_f_vector(self, size_t num_threads, size_t parallelization_depth, int dual) except -1:
Expand All @@ -3497,7 +3491,6 @@ cdef class CombinatorialPolyhedron(SageObject):
return 0 # There is no need to recompute the f_vector.

cdef int dim = self.dimension()
cdef int d # dimension of the current face of the iterator
cdef MemoryAllocator mem = MemoryAllocator()

if num_threads == 0:
Expand Down Expand Up @@ -3686,7 +3679,7 @@ cdef class CombinatorialPolyhedron(SageObject):

from sage.arith.misc import binomial
estimate_n_faces = self.dimension() * binomial(min(self.n_facets(), self.n_Vrepresentation()),
self.dimension() // 2)
self.dimension() // 2)

# Note that the runtime per face already computes the coatoms of the next level, i.e.
# the runtime for each facet suffices to compute all ridges in primal,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ from sage.geometry.polyhedron.combinatorial_polyhedron.list_of_faces
from sage.geometry.polyhedron.combinatorial_polyhedron.face_data_structure cimport face_t
from sage.geometry.polyhedron.combinatorial_polyhedron.face_iterator cimport FaceIterator


@cython.final
cdef class CombinatorialFace(SageObject):
cdef readonly bint _dual # if 1, then iterate over dual Polyhedron
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,25 +173,25 @@ cdef class CombinatorialFace(SageObject):

# Copy data from FaceIterator.
it = data
self._dual = it.dual
self.atoms = it.atoms
self.coatoms = it.coatoms
self._dual = it.dual
self.atoms = it.atoms
self.coatoms = it.coatoms

if it.structure.face_status == FaceStatus.NOT_INITIALIZED:
raise LookupError("face iterator not set to a face")

face_init(self.face, self.coatoms.n_atoms(), self.coatoms.n_coatoms())
face_copy(self.face, it.structure.face)

self._dimension = it.structure.current_dimension
self._dimension = it.structure.current_dimension
self._ambient_dimension = it.structure.dimension
self._ambient_Vrep = it._Vrep
self._ambient_facets = it._facet_names
self._n_ambient_facets = it._n_facets
self._equations = it._equations
self._n_equations = it._n_equations
self._hash_index = it.structure._index
self._ambient_bounded = it._bounded
self._ambient_Vrep = it._Vrep
self._ambient_facets = it._facet_names
self._n_ambient_facets = it._n_facets
self._equations = it._equations
self._n_equations = it._n_equations
self._hash_index = it.structure._index
self._ambient_bounded = it._bounded

self._initialized_from_face_lattice = False

Expand All @@ -203,35 +203,35 @@ cdef class CombinatorialFace(SageObject):
assert 0 <= index < all_faces.f_vector[dimension + 1], "index is out of range"

# Copy data from PolyhedronFaceLattice.
self._dual = all_faces.dual
self.atoms = all_faces.atoms
self.coatoms = all_faces.coatoms
self._dual = all_faces.dual
self.atoms = all_faces.atoms
self.coatoms = all_faces.coatoms

face_init(self.face, self.coatoms.n_atoms(), self.coatoms.n_coatoms())
face_copy(self.face, all_faces.faces[dimension+1].faces[index])

self._dimension = dimension
self._dimension = dimension
self._ambient_dimension = all_faces.dimension
self._ambient_Vrep = all_faces._Vrep
self._ambient_facets = all_faces._facet_names
self._equations = all_faces._equations
self._n_equations = len(self._equations) if self._equations else 0
self._ambient_Vrep = all_faces._Vrep
self._ambient_facets = all_faces._facet_names
self._equations = all_faces._equations
self._n_equations = len(self._equations) if self._equations else 0
if self._dual:
self._n_ambient_facets = self.atoms.n_faces()
else:
self._n_ambient_facets = self.coatoms.n_faces()
self._ambient_bounded = all_faces._bounded
self._ambient_bounded = all_faces._bounded

self._initialized_from_face_lattice = True

self._hash_index = index
for i in range(-1,dimension):
for i in range(-1, dimension):
self._hash_index += all_faces.f_vector[i+1]

# Add the complete ``f-vector`` to the hash index,
# such that hash values obtained by an iterator or by the face lattice
# do not collide.
for i in range(-1,self._ambient_dimension+1):
for i in range(-1, self._ambient_dimension+1):
self._hash_index += all_faces.f_vector[i+1]
else:
raise ValueError("data must be face iterator or a list of all faces")
Expand Down Expand Up @@ -273,7 +273,7 @@ cdef class CombinatorialFace(SageObject):
'A 3-dimensional face of a 5-dimensional combinatorial polyhedron'
"""
return "A {}-dimensional face of a {}-dimensional combinatorial polyhedron"\
.format(self.dimension(), self.ambient_dimension())
.format(self.dimension(), self.ambient_dimension())

def __reduce__(self):
r"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,15 +58,15 @@ AUTHOR:
- Jonathan Kliem (2019-04)
"""

#*****************************************************************************
# ****************************************************************************
# Copyright (C) 2019 Jonathan Kliem <jonathan.kliem@gmail.com>
#
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation, either version 2 of the License, or
# (at your option) any later version.
# http://www.gnu.org/licenses/
#*****************************************************************************
# https://www.gnu.org/licenses/
# ****************************************************************************

from memory_allocator cimport MemoryAllocator

Expand Down Expand Up @@ -94,6 +94,7 @@ def _Vrep_list_to_bit_rep_wrapper(tup):
Vrep_list_to_bit_rep(tup, output.data.faces[0])
return output


cdef int Vrep_list_to_bit_rep(tuple Vrep_list, face_t output) except -1:
r"""
Convert a vertex list into Bit-representation. Store it in ``output``.
Expand Down Expand Up @@ -135,6 +136,7 @@ cdef int Vrep_list_to_bit_rep(tuple Vrep_list, face_t output) except -1:
for entry in Vrep_list:
face_add_atom_safe(output, entry)


def _incidences_to_bit_rep_wrapper(tup):
r"""
A function to allow doctesting of :func:`incidences_to_bit_rep`.
Expand All @@ -150,6 +152,7 @@ def _incidences_to_bit_rep_wrapper(tup):
incidences_to_bit_rep(tup, output.data.faces[0])
return output


cdef int incidences_to_bit_rep(tuple incidences, face_t output) except -1:
r"""
Convert a tuple of incidences into Bit-representation.
Expand Down Expand Up @@ -185,6 +188,7 @@ cdef int incidences_to_bit_rep(tuple incidences, face_t output) except -1:
# Vrep ``entry`` is contained in the face, so set the corresponding bit
face_add_atom_safe(output, entry)


def incidence_matrix_to_bit_rep_of_facets(Matrix_dense matrix):
r"""
Initialize facets in Bit-representation as :class:`~sage.geometry.polyhedron.combinatorial_polyhedron.list_of_faces.ListOfFaces`.
Expand Down Expand Up @@ -249,6 +253,7 @@ def incidence_matrix_to_bit_rep_of_facets(Matrix_dense matrix):
face_add_atom_safe(output, entry)
return facets


def incidence_matrix_to_bit_rep_of_Vrep(Matrix_dense matrix):
r"""
Initialize Vrepresentatives in Bit-representation as :class:`~sage.geometry.polyhedron.combinatorial_polyhedron.list_of_faces.ListOfFaces`.
Expand Down Expand Up @@ -307,6 +312,7 @@ def incidence_matrix_to_bit_rep_of_Vrep(Matrix_dense matrix):
"""
return incidence_matrix_to_bit_rep_of_facets(matrix.transpose())


def facets_tuple_to_bit_rep_of_facets(tuple facets_input, size_t n_Vrep):
r"""
Initializes facets in Bit-representation as :class:`~sage.geometry.polyhedron.combinatorial_polyhedron.list_of_faces.ListOfFaces`.
Expand Down Expand Up @@ -348,6 +354,7 @@ def facets_tuple_to_bit_rep_of_facets(tuple facets_input, size_t n_Vrep):
facet_set_coatom(facets.data.faces[i], i)
return facets


def facets_tuple_to_bit_rep_of_Vrep(tuple facets_input, size_t n_Vrep):
r"""
Initialize Vrepresentatives in Bit-representation as :class:`~sage.geometry.polyhedron.combinatorial_polyhedron.list_of_faces.ListOfFaces`.
Expand Down Expand Up @@ -408,6 +415,7 @@ def facets_tuple_to_bit_rep_of_Vrep(tuple facets_input, size_t n_Vrep):
face_add_atom_safe(Vrep_data[input_Vrep], input_facet)
return Vrep


def _bit_rep_to_Vrep_list_wrapper(ListOfFaces faces, index=0):
r"""
A function to test :func:`bit_rep_to_Vrep_list`.
Expand All @@ -433,10 +441,10 @@ def _bit_rep_to_Vrep_list_wrapper(ListOfFaces faces, index=0):
output = <size_t *> mem.allocarray(faces.n_atoms(),
sizeof(size_t))

length = bit_rep_to_Vrep_list(
faces.data.faces[index], output)
length = bit_rep_to_Vrep_list(faces.data.faces[index], output)
return tuple(output[i] for i in range(length))


cdef inline size_t bit_rep_to_Vrep_list(face_t face, size_t *output) except -1:
r"""
Convert a bitrep-representation to a list of Vrep. Return length of representation.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,12 @@ cdef class FaceIterator_base(SageObject):
cdef int only_subsets(self) except -1
cdef int find_face(self, face_t face) except -1


@cython.final
cdef class FaceIterator(FaceIterator_base):
pass


@cython.final
cdef class FaceIterator_geom(FaceIterator_base):
cdef int _trivial_faces # Whether to yield the trivial faces.
Expand Down
Loading

0 comments on commit 69d03d3

Please sign in to comment.