Skip to content

ENH: Refactor CHOLMOD interface#148

Merged
broesler merged 163 commits into
scikit-sparse:masterfrom
broesler:cholmod-refactor
Feb 11, 2026
Merged

ENH: Refactor CHOLMOD interface#148
broesler merged 163 commits into
scikit-sparse:masterfrom
broesler:cholmod-refactor

Conversation

@broesler
Copy link
Copy Markdown
Collaborator

This PR is a major re-write of the CHOLMOD interface Cython code and API to reflect the most up-to-date Cython version (>=3.1) and be more consistent with existing SciPy interfaces like scipy.linalg.cholesky and scipy.linalg.cho_factor. It is a breaking change, so the version has been bumped to v0.5.0.

Changes include:

  • Major API updates to the sksparse.cholmod module. The module has been
    updated to resemble the existing scipy.linalg.cholesky interface, as well as
    provide additional functions present in the SuiteSparse CHOLMOD MATLAB
    interface, and MATLAB built-in Cholesky functions. The underlying Cython code
    has been refactored to provide greater type safety and performance.
    • The cholmod.Factor class has been renamed to
      sksparse.cholmod.CholeskyFactor.
    • The cholmod.Common class has been removed. Its attributes have been
      subsumed into the CholeskyFactor class.
    • The sksparse.cholmod.cholesky function now returns
      a ~scipy.sparse.csc_array instead of a Factor object, and an optional
      ~numpy.ndarray containing the permutation vector.
    • The sksparse.cholmod.ldl function has been added. It returns a tuple
      (~scipy.sparse.csc_array, ~scipy.sparse.csc_array), and an optional
      ~numpy.ndarray containing the permutation vector.
    • A sksparse.cholmod.cho_factor function has been added to perform the
      numeric Cholesky factorization and return
      a sksparse.cholmod.CholeskyFactor object.
    • Similarly, a sksparse.cholmod.ldl_factor function has been added to
      perform the numeric LDL factorization and return
      a sksparse.cholmod.CholeskyFactor object.
    • The cholmod.analyze function has been removed. The analysis step is now
      performed when calling the constructor of sksparse.cholmod.CholeskyFactor.
    • The use_long parameter has been removed from the
      sksparse.cholmod.cholesky and sksparse.cholmod.ldl functions. The type
      of indices is now inferred from the input matrix.
    • The mode parameter has been renamed to supernodal_mode.
    • The symmetric parameter has been removed. It has been replaced by two new
      parameters: lower, and sym_kind.
      • Parameter lower controls whether to use the lower or upper triangular
        part of the input matrix, or whether to return a lower or upper triangular
        factor.
      • Parameter sym_kind has been added. It accepts a string argument in ``,
        which controls the symmetry structure of the matrix to analyze.
    • The functions cholmod.analyze_AAt and cholmod.cholesky_AAt have been
      removed. Use sksparse.cholmod.cho_factor or sksparse.cholmod.cholesky
      with sym_kind="row" instead.
    • The ordering_method parameter has been renamed to order.
    • The Factor methods L, D, LD, L_D, and P, have been removed in
      factor of the methods sksparse.cholmod.CholeskyFactor.get_factor and
      sksparse.cholmod.CholeskyFactor.get_perm.
    • The property perm and method sksparse.cholmod.CholeskyFactor.factor have
      been added to return read-only views of the permutation vector and factor
      matrix, respectively.
    • The Factor methods solve_LDLt, solve_LD, solve_DLt, solve_L,
      solve_Lt, and solve_D have been removed in favor of the single
      sksparse.cholmod.CholeskyFactor.solve method. The
      sksparse.cholmod.CholeskyFactor is not callable.
    • Add multiple properties to the sksparse.cholmod.CholeskyFactor class for
      convenient access to cholmod_factor attributes. See the full documentation
      for details.
    • Fix a bug in the previous version where sparse inputs with inconsistent
      has_sorted_indices or has_canonical_format flags would silently lead to
      incorrect results. The input matrix is now modified into a canonical CSC
      format, regardless of the input format.

Now, rst directives like ":mod:`scipy.sparse`" etc. will link to the
appropriate readthedocs pages for those modules.
Use ruff check and build docs with SPHINXOPTS=-n for "nitpicky". Update
many of the `` :meth:`.Factor.solve_P` `` directives to use the leading
dot in order to resolve the link properly.
This line causes a warning (thus a CI failure) when we run:

    make SPHINXOPTS=-nW html

so just comment it out since the directory is unused.
The word "Sparse" is unnecessary in the title since the whole package is
for sparse matrices. Move the reference to `sksparse.cholmod` into the
first paragraph, and remove the unnecessary "Overview" title.
No need to explicitly list the signatures. That's what auto does.
* Sentences shouldn't start with "However,".
* Un-parenthesize phrases that don't need it.
* i.e. and e.g. are italicized (Latin phrases).
* Prefer double quotes over single quotes in prose.
* Italicize mathematical variables like A, L, or D.
* Fix some "math:`x`" -> ":math:`x`".
* Use either double backticks ("``x``") or ":code:`x`" for fixed-width
  font. Single backticks renders as italics.
Page title reflects the more verbose description (e.g. "Cholesky
Decomposition (sksparse.cholmod)").
Will be used in all other features, so create it here for cholmod first.
The function does *not* accept csc_matrix inputs (deprecated API), so be
explicit about the input not being in csc_array format.
Bump version to 0.5.0.

Implement cholesky python function to match chol2.m (chol2.c mex).
The exposed API is different, but many of the underlying functions are
the same. There are now more type-safe versions than taking raw
np.ndarray.data pointers.

Move cholmod.pyx -> _cholmod_internal.pyx.

This commit also moves the existing code and updates the docs to have
`cholmod` -> `_cholmod_internal`. Future commits will update the entire
cholmod API to more closely align with the MATLAB interface of CHOLMOD.
Checkout the file, and make these changes:
* Add pos_def_only kwarg.
* Add dtype kwarg.
* Update pos_def_only to use conjugate transpose for complex
  matrices.
* Make random matrices *exactly* Hermitian if pos_def_only=True.
* Test cholesky on random real matrices.
* Test cholesky ordering methods.
* Test cholesky on real and complex dtypes.
* Remove old redundant tests in sksparse/test*
* Refactor cholesky to use validate_csc_input.
* Free memory at end of cholesky.
* Use `order=None` as default. Refactor itype test out of class.

The API is more intuitive with None as the default. Now, `order=None`
uses natural ordering, but does not return a permutation vector. An
explicit `order="natural"` (or any other option) will return
a permutation vector.
This function is largely the same as that used in the original
cholmod.pyx, but is now safe for use with a `noexcept` C interface,
since we just check for and handle the errors at the python level.

The "#cython: legacy_implicit_noexcept = True" line is not recommended
in modern code, so using `cholmod_common.error_handler` is not possible
(and not necessary) in our updated interface.

Add more informative error messages.
The constant defined in the did not work, and was defaulted to 0.
Instead, define the constants in the pyx file.
NOTE that some still fail for complex dtypes. There is likely a bug in
the _cholmod_sparse_from_csc function dtype handline.
Also clean up data type parsing in scipy -> cholmod conversion.
This behavior is consistent with the CHOLMOD/MATLAB cholmod2.c and
ldlsolve.c interfaces.
This commit adds lines to the `validate_csc_input` function that force
an in-place change of the input matrix to ensure that there are no
duplicates and the indices are sorted.

Add tests for non-canonical input (as of scipy v1.16.2), and ensure
`davis_example_chol` is returned as canonical format to isolate testing.
Replace instances of `self._factor.xtype != CHOLMOD_PATTERN` with more
read-able public property.
The input matrix must use the same symmetry/triangle as the originally
analyzed matrix, so don't allow users to override that in the numerical
factorization function.
Depends on numpy version, so make the tests more robust.
The error_map -> _ERROR_MAP is a constant, so declare it outside of the
function. Also use `int _handle_errors(...) except -1` instead of
`except *` for speed.
Include `cimport numpy as cnp` as additional alias to `np` for now. Will
incrementally update references and remove `cimport numpy as np`
eventually.
This commit uses fused types with memoryviews onto underlying arrays. It
removes the need to return a python object reference to the underlying
arrays, and type checks the input array. It saves many lines of manual
type-checking code.

This commit also creates templated functions that return the appropriate
CHOLMOD_SINGLE etc. xtype and dtype ints based on the fused type
selected.
This change allows Cython to avoid jumping back to Python for basic
integer lookups. The integer values are #defined in the cholmod.h
header, so we don't want to use an array in case they change, but the
if-statements compile well.
This commit makes many minor changes to ensure more "white" code in the
annotated Cythonize output:
  * Marks some pure C functions "noexcept"
  * Sets appropriate "except -1"
  * Removes unnecessary `np.ndarray` typing
  * Adds types to other variables to minimize Python interaction where
    possible
  * Directly use underlying C members to avoid `@property` overhead.
  * Use `Py_ssize_t` type for sizes of Python arrays.
This commit makes the behavior of cholmod more like
`scipy.linalg.cholesky` or `scipy.linalg.lu`, which accept bool and int
matrices, but convert them to float. It is also consistent with
the CHOLMOD MATLAB interface (although MATLAB does not support sparse
integer matrices).
This change leaves CholeskyFactor in an "unsafe" state after __cinit__
is run (or `CholeskyFactor.__new__(CholeskyFactor)`), since the `_cm`
and `_factor` pointers are NULL, but it would have to be very
intentional on the part of the user to create an object as such. There
is a warning in the docs.
Just use the __dealloc__ method directly.
@broesler broesler mentioned this pull request Dec 12, 2025
@broesler broesler merged commit 19a05e5 into scikit-sparse:master Feb 11, 2026
@broesler broesler deleted the cholmod-refactor branch February 11, 2026 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant