Optional SparseMatrixCSR support via SparseMatricesCSR extension#36
Draft
ChrisRackauckas-Claude wants to merge 3 commits into
Draft
Optional SparseMatrixCSR support via SparseMatricesCSR extension#36ChrisRackauckas-Claude wants to merge 3 commits into
ChrisRackauckas-Claude wants to merge 3 commits into
Conversation
The entire kernel is already CSC: csr_analyze/csr_factor/csr_refactor!/csr_qr now accept a SparseMatrixCSC directly, reading its colptr/rowval/nzval into the pooled workspace with no transpose or intermediate allocation (column norms are computed straight off nzval). The csr_* names are kept for back-compatibility. SparseMatricesCSR is moved from [deps] to [weakdeps]; the SparseMatrixCSR method overloads now live in SparseColumnPivotedQRSparseMatricesCSRExt, which delegates to the CSC core via SparseMatrixCSC(A). The precompile workload exercises the CSC path natively; the CSR extension carries its own workload. Tests: CSC core covers square/over-/under-determined/rank-deficient, Float64 and ComplexF64, and the refactor reuse + zero-alloc steady state, run in a SparseMatricesCSR-free subprocess. The existing CSR suite still exercises the extension path. Refactor steady state on the CSC path is 0 bytes (sparse and adaptive_dense), as is ldiv!. Refs SciML#33 Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Remove the optional SparseMatricesCSR extension and all SparseMatricesCSR references so the package has no dependency on it. The csr_* public names are kept (they accept SparseMatrixCSC). CSR (SparseMatrixCSR) input support is carved into a separate, optional PR stacked on this one. - Delete ext/SparseColumnPivotedQRSparseMatricesCSRExt.jl. - Remove SparseMatricesCSR from [weakdeps]/[extensions]/[compat]/[extras]/[targets]. - Make the tests CSC-only (helpers now return SparseMatrixCSC); fold the CSC-core checks into a plain include (no subprocess isolation needed once CSR can never be loaded). - Scrub CSR mentions from src docstrings, the precompile comment, README, and docs. Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Stacked on the CSC-native core (SciML#35). Re-adds SparseMatrixCSR input support behind the optional SparseMatricesCSR weak-dep extension, which converts CSR to CSC and dispatches into the unchanged CSC kernel. The package's core stays pure CSC + SparseArrays; CSR is only pulled in when `using SparseMatricesCSR`. - Restore ext/SparseColumnPivotedQRSparseMatricesCSRExt.jl (csr_qr/csr_analyze/ csr_factor/csr_refactor! on SparseMatrixCSR, plus its own precompile workload). - Restore SparseMatricesCSR in [weakdeps]/[extensions]/[compat]/[extras]/[targets]. - Restore the CSR tests (including the subprocess check that the CSC core works with the CSR extension absent) and the CSR docstring/precompile notes, README, and docs. Note: the current CSR path reconverts CSR -> CSC on every csr_refactor! call (~17 KB/call, ~1.8x slower; see the SciML#35 benchmark comment). A future revision should cache a CSC buffer and reuse it, and ideally add a zero-copy CSR fast path (SparseMatrixCSR(A) == SparseMatrixCSC(Aᵀ), so the CSR arrays can be reinterpreted as the transpose's CSC with no copy). Not implemented here. Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
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
2 participants
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.
Stacked on #35; optional — please ignore until reviewed by @ChrisRackauckas. This may not be merged.
Re-adds optional
SparseMatrixCSRinput support on top of the CSC-native core (#35), behind theSparseMatricesCSRweak-dep extension. The package core stays pureSparseMatrixCSC+SparseArrays;SparseMatricesCSRis only loaded when the user doesusing SparseMatricesCSR. The extension converts CSR → CSC and dispatches into the unchanged CSC kernel.Status / caveats (not addressed here — captured as TODO)
Per-call reallocation. The current CSR path reconverts CSR →
SparseMatrixCSCon everycsr_refactor!call (~17 KB/call, ~1.8× slower than the native CSC path — see the benchmark comment on Make SparseMatrixCSC the native API #35). If this PR is pursued,csr_refactor!on aSparseMatrixCSRshould reuse a cached CSC buffer instead of reconverting each call, so the refactor stays allocation-light like the native CSC path.Zero-copy CSR / transpose fast-path (the principled motivation). CSR is the natural representation for transpose/adjoint factorization:
SparseMatrixCSR(A)is structurally identical toSparseMatrixCSC(Aᵀ)(samerowptr/colval/nzval↔colptr/rowval/nzvalarrays). So a future zero-copy fast path could reinterpret the CSR arrays as the transpose's CSC with no copy, giving a genuine transpose-QR entry point rather than a convert-and-forward shim. That is the real reason to keep CSR around.Neither optimization is implemented here — this PR just carries the extension as-is so the option is preserved.
Tests
Pkg.test()): 730 + 24 + 1 = 755 passing, including the subprocess check that the CSC core still works with the CSR extension absent.🤖 Generated with Claude Code