ENH: Refactor CHOLMOD interface#148
Merged
Merged
Conversation
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.
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.choleskyandscipy.linalg.cho_factor. It is a breaking change, so the version has been bumped to v0.5.0.Changes include:
sksparse.cholmodmodule. The module has beenupdated to resemble the existing
scipy.linalg.choleskyinterface, as well asprovide 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.
cholmod.Factorclass has been renamed tosksparse.cholmod.CholeskyFactor.cholmod.Commonclass has been removed. Its attributes have beensubsumed into the
CholeskyFactorclass.sksparse.cholmod.choleskyfunction now returnsa
~scipy.sparse.csc_arrayinstead of aFactorobject, and an optional~numpy.ndarraycontaining the permutation vector.sksparse.cholmod.ldlfunction has been added. It returns a tuple(
~scipy.sparse.csc_array,~scipy.sparse.csc_array), and an optional~numpy.ndarraycontaining the permutation vector.sksparse.cholmod.cho_factorfunction has been added to perform thenumeric Cholesky factorization and return
a
sksparse.cholmod.CholeskyFactorobject.sksparse.cholmod.ldl_factorfunction has been added toperform the numeric LDL factorization and return
a
sksparse.cholmod.CholeskyFactorobject.cholmod.analyzefunction has been removed. The analysis step is nowperformed when calling the constructor of
sksparse.cholmod.CholeskyFactor.use_longparameter has been removed from thesksparse.cholmod.choleskyandsksparse.cholmod.ldlfunctions. The typeof indices is now inferred from the input matrix.
modeparameter has been renamed tosupernodal_mode.symmetricparameter has been removed. It has been replaced by two newparameters:
lower, andsym_kind.lowercontrols whether to use the lower or upper triangularpart of the input matrix, or whether to return a lower or upper triangular
factor.
sym_kindhas been added. It accepts a string argument in ``,which controls the symmetry structure of the matrix to analyze.
cholmod.analyze_AAtandcholmod.cholesky_AAthave beenremoved. Use
sksparse.cholmod.cho_factororsksparse.cholmod.choleskywith
sym_kind="row"instead.ordering_methodparameter has been renamed toorder.FactormethodsL,D,LD,L_D, andP, have been removed infactor of the methods
sksparse.cholmod.CholeskyFactor.get_factorandsksparse.cholmod.CholeskyFactor.get_perm.permand methodsksparse.cholmod.CholeskyFactor.factorhavebeen added to return read-only views of the permutation vector and factor
matrix, respectively.
Factormethodssolve_LDLt,solve_LD,solve_DLt,solve_L,solve_Lt, andsolve_Dhave been removed in favor of the singlesksparse.cholmod.CholeskyFactor.solvemethod. Thesksparse.cholmod.CholeskyFactoris not callable.sksparse.cholmod.CholeskyFactorclass forconvenient access to
cholmod_factorattributes. See the full documentationfor details.
has_sorted_indicesorhas_canonical_formatflags would silently lead toincorrect results. The input matrix is now modified into a canonical CSC
format, regardless of the input format.