Accept AbstractSparseMatrixCSC for decompression by blegat · Pull Request #298 · JuliaDiff/SparseMatrixColorings.jl · GitHub

Here is a suggestion for #296 (comment)
I can write tests if the change sounds reasonable

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.52%. Comparing base (0996030) to head (e93a786).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #298      +/-   ##
==========================================
- Coverage   99.39%   95.52%   -3.87%     
==========================================
  Files          20       20              
  Lines        2145     2145              
==========================================
- Hits         2132     2049      -83     
- Misses         13       96      +83

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

I don't think there are any API specifications on SparseArrays.AbstractSparseMatrixCSC, and since we make heavy use of the fields of SparseMatrixCSC, this seems like a risky move. Can you explain where it comes in handy?

Yes, the decompression relies on a few properties of SparseMatrixCSC. It seems that it needs to be able to

Get the size
Get colptr, currently it is accessing it directly via the field but it could call getcolptr instead
Call nonzeros

In my case, the nonzeros is a view and I want to avoid having to copy it to a Vector so that it fits in a SparseMatrixCSC. So I define a custom AbstractSparseMatrixCSC:
https://github.com/blegat/ArrayDiff.jl/pull/23/files#diff-47c27891e951c8cd946b850dc2df31082624afdf57446c21cb6992f5f4b74aa2R207-R215

The properties that the implementation relies on are that getcolptr is the colptr if a CSC representation, that seems safe to assume and that the nonzeros are sorted in increasing row indices (per column), which is perhaps the one more risky but we may also just consider that it's part of the definition of the CSC format.
The other option would be defining a custom struct in this package (so basically just copy-pasting the one I did in ArrayDiff). Then there is the risk of someone needed yet another struct.
Both options look fine to me, let me know which one you prefer ;)

The properties that the implementation relies on are that getcolptr is the colptr if a CSC representation, that seems safe to assume and that the nonzeros are sorted in increasing row indices (per column), which is perhaps the one more risky but we may also just consider that it's part of the definition of the CSC format.

There are CSC matrices in the ecosystem where the sorting is not enforced, see eg JuliaGPU/GPUArrays.jl#648. The question is whether there could be subtypes of SparseArrays.AbstractSparseMatrixCSC where that is also the case, and the answer is probably: since there is no documentation, anything goes?

Given that uncertainty, I kinda like the suggestion you made in #296 to split out a decompression subroutine that uses an AbstractVector (probably with 1-based, contiguous indexing checked upfront).

Reading more the implementation of the decompression, it seemed that it was also using the colptr vector while I initially though it was only using the nonzeros when I wrote that comment. Hence the change of mind with this PR. The implementation of the decompression is however not reading the rowvals since it assumes that it is sorted. So one option could be to pass along both the colptr and the nonzeros.
Another alternative is for SMC to define a new function is_rowvals_increasing_per_column with a default implementation returning false, implement it for SparseMatrixCSC and then ArrayDiff could implement it as well for its custom sparse matrix. The, we can check it and error if it returns false. Let me know what you prefer

I think I'd rather add an internal indirection step like

function decompress!(A::SparseMatrixCSC, ...)
    return decompress_csc!(A.nzval, A.rowval, A.colptr, A.m, A.n, ...)
end

than rely on an incompletely specified AbstractSparseMatrixCSC API. You can then use decompress_csc! in ArrayDiff, until we figure out something cleaner?

Co-authored-by: Guillaume Dalle <22795598+gdalle@users.noreply.github.com>

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.20%. Comparing base (f56da29) to head (2fd25ee).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #298      +/-   ##
==========================================
+ Coverage   95.52%   99.20%   +3.68%     
==========================================
  Files          20       20              
  Lines        2145     2145              
==========================================
+ Hits         2049     2128      +79     
+ Misses         96       17      -79

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Thanks! Since this only affects private internals, I see no reason to delay it any further. I'll merge and release so that you may experiment with it, then we'll see whether we need to extend what you coded to other result types

* Bump julia-actions/julia-downgrade-compat from 1 to 2 (#258) Bumps [julia-actions/julia-downgrade-compat](https://github.com/julia-actions/julia-downgrade-compat) from 1 to 2. - [Release notes](https://github.com/julia-actions/julia-downgrade-compat/releases) - [Commits](julia-actions/julia-downgrade-compat@v1...v2) --- updated-dependencies: - dependency-name: julia-actions/julia-downgrade-compat dependency-version: '2' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump actions/checkout from 4 to 5 (#259) * ensure `UnsupportedDecompressionError <: Exception` (#260) Not sure if there is any benefit, but I suppose the intention is for this type, as an exception type, to subtype `Exception`. * Bump peter-evans/create-or-update-comment from 4 to 5 (#261) * Bump peter-evans/find-comment from 3 to 4 (#262) Bumps [peter-evans/find-comment](https://github.com/peter-evans/find-comment) from 3 to 4. - [Release notes](https://github.com/peter-evans/find-comment/releases) - [Commits](peter-evans/find-comment@v3...v4) --- updated-dependencies: - dependency-name: peter-evans/find-comment dependency-version: '4' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Allow picking the best of several orders for GreedyColoringAlgorithm (#265) * Allow picking the best of several orders for GreedyColoringAlgorithm * Fix type params * Tuple * Better tests * Better test * Fix foc * Fix type inference inside closure * Fix seed * Test on 1.11 * Avoid duplicate remap_colors * chore: bump version to 0.4.22 (#268) * chore: fix typo in package UUID (#269) * Make any coloring algorithm compatible with SMC (#263) * Make any coloring algorithm compatible with SMC * Fix StackOverflow * Run tests on 1.11 * Start working on optimal coloring * Fix tests * Improve feasibility check * Format * Fix * Fix * Typo * Use HiGHS * Remove OptimalColoringAlgorithm * Remove test deps * Don't handle forced colors in acyclic * Run GPU CI on 1.11 too * Actually use forced colors * Format * Deactivate JuliaFormatter (can't figure out why it fails) * Optimal coloring algorithm with JuMP formulation (#271) * Add a file postprocessing.jl (#275) * Rename has_diagonal into augmented_graph (#273) * Rename has_diagonal into augmented_graph * Fix a typo in postprocessing.jl * Update src/graph.jl Co-authored-by: Guillaume Dalle <22795598+gdalle@users.noreply.github.com> --------- Co-authored-by: Guillaume Dalle <22795598+gdalle@users.noreply.github.com> * Try tests on Julia 1.12 (#276) * Bump SMC to v0.4.23 (#277) * Patch failing alloc test (#278) * Add postprocess_with_star_set! and postprocess_with_tree_set! (#279) * Bump actions/checkout from 5 to 6 (#282) Bumps [actions/checkout](https://github.com/actions/checkout) from 5 to 6. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](actions/checkout@v5...v6) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: '6' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Replace is_solved_and_feasible with assert_is_solved_and_feasible (#284) * Fix JET tests (#285) * Fix JET tests * Move JuliaFormatter to action * Fix formatting * Use diagonal_indices in the general decompress! for acyclic coloring (#287) * Enhance decompress! for bicoloring (#288) * Add a compat entry for CUDA.jl in test/Project.toml (#293) * Add nb_self_loops in AdjacencyGraph (#290) * Remove unused arguments from internal functions (#295) * Add test functions substitutable_columns and substitutable_bidirectional (#297) * Document fields of TreeSet (#296) * Fix respectful_similar with SparsityPatternCSC (#299) * Fix respectful_similar with SparsityPatternCSC * Add test * Fix format * Add test --------- Co-authored-by: Alexis Montoison <35051714+amontoison@users.noreply.github.com> * Fix coloring with empty matrix as input (#300) * Fix coloring with empty matrix as input * Update src/result.jl Co-authored-by: Guillaume Dalle <22795598+gdalle@users.noreply.github.com> * Add check * Fix format * Change check * Apply suggestion from @gdalle --------- Co-authored-by: Guillaume Dalle <22795598+gdalle@users.noreply.github.com> * test: skip failing MiniZinc tests (#305) * Fix eltype invalidation for SparsityPatternCSC (#304) SparsityPatternCSC{Ti} <: AbstractMatrix{Bool} but the custom Base.eltype method was returning Ti (the index type) instead of Bool. This caused 24,906 method invalidations when loading the package, as it invalidated the backedge from Base.eltype(::AbstractArray). Change the custom method to SparseArrays.indtype instead, since that's what the type parameter Ti actually represents. The eltype is now correctly inherited from AbstractMatrix{Bool}. Co-authored-by: ChrisRackauckas-Claude <accounts@chrisrackauckas.com> Co-authored-by: Guillaume Dalle <22795598+gdalle@users.noreply.github.com> * chore: bump version (#306) * Bump julia-actions/cache from 2 to 3 (#307) * Update repo owner to JuliaDiff org (#309) * Update repo owner * Bump version * Switch GPU CI to buildkite * Add badge * Fix buildkite badge [skip tests] (#310) * Accept AbstractSparseMatrixCSC for decompression (#298) * Accept AbstractSparseMatrixCSC for decompression * Define decompress_csc * Remove unused import of AbstractSparseMatrixCSC * Apply suggestion from @blegat * Change return statement to return nothing * Apply suggestion from @blegat * Add to docs * Apply suggestion from @gdalle Co-authored-by: Guillaume Dalle <22795598+gdalle@users.noreply.github.com> * Move to internals --------- Co-authored-by: Guillaume Dalle <22795598+gdalle@users.noreply.github.com> * Bump version (#311) * Bump codecov/codecov-action from 5 to 6 (#312) Bumps [codecov/codecov-action](https://github.com/codecov/codecov-action) from 5 to 6. - [Release notes](https://github.com/codecov/codecov-action/releases) - [Changelog](https://github.com/codecov/codecov-action/blob/main/CHANGELOG.md) - [Commits](codecov/codecov-action@v5...v6) --- updated-dependencies: - dependency-name: codecov/codecov-action dependency-version: '6' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore: bump CUDA compat to v6 (cuSPARSE now a separate package) (#314) * chore: bump CUDA compat to v6 (cuSPARSE now a separate package) * More renaming * Format * Fix compression? * Generic compression and result * Import * No generic result * Fix ambiguity * Bump julia-actions/setup-julia from 2 to 3 (#315) Bumps [julia-actions/setup-julia](https://github.com/julia-actions/setup-julia) from 2 to 3. - [Release notes](https://github.com/julia-actions/setup-julia/releases) - [Commits](julia-actions/setup-julia@v2...v3) --- updated-dependencies: - dependency-name: julia-actions/setup-julia dependency-version: '3' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Neven Sajko <4944410+nsajko@users.noreply.github.com> Co-authored-by: Guillaume Dalle <22795598+gdalle@users.noreply.github.com> Co-authored-by: Alexis Montoison <35051714+amontoison@users.noreply.github.com> Co-authored-by: Benoît Legat <benoit.legat@gmail.com> Co-authored-by: Chris Rackauckas - Beep Boop Edition <admin@chrisrackauckas.com> Co-authored-by: ChrisRackauckas-Claude <accounts@chrisrackauckas.com>

This was referenced Jan 19, 2026

Replace Coloring by SparseMatrixColoring blegat/ArrayDiff.jl#19

Open

Simplify hessian coloring blegat/ArrayDiff.jl#23

Open

gdalle reviewed Jan 19, 2026

View reviewed changes

blegat requested a review from gdalle February 12, 2026 15:07

blegat force-pushed the abstractsparse branch from 6a8ec50 to a4ed265 Compare February 20, 2026 16:21

blegat commented Feb 27, 2026

View reviewed changes

Comment thread src/matrices.jl Outdated

blegat commented Feb 27, 2026

View reviewed changes

Comment thread src/decompression.jl Outdated

blegat added 7 commits March 3, 2026 12:18

Accept AbstractSparseMatrixCSC for decompression

22b37fd

Define decompress_csc

bd06fd3

Remove unused import of AbstractSparseMatrixCSC

c351e79

Apply suggestion from @blegat

b8451af

Change return statement to return nothing

d4a736a

Apply suggestion from @blegat

6ad9a2f

Add to docs

a029f83

blegat force-pushed the abstractsparse branch from aa079b5 to a029f83 Compare March 3, 2026 11:18

gdalle requested changes Mar 9, 2026

View reviewed changes

Comment thread docs/src/api.md Outdated

Comment thread src/decompression.jl Outdated

Comment thread src/decompression.jl

blegat and others added 2 commits March 9, 2026 15:07

Apply suggestion from @gdalle

e93a786

Co-authored-by: Guillaume Dalle <22795598+gdalle@users.noreply.github.com>

Merge branch 'main' into abstractsparse

9469e4d

Move to internals

2fd25ee

gdalle approved these changes Mar 20, 2026

View reviewed changes

gdalle merged commit 9aebbe6 into JuliaDiff:main Mar 20, 2026
9 checks passed

Accept AbstractSparseMatrixCSC for decompression by blegat · Pull Request #298 · JuliaDiff/SparseMatrixColorings.jl · GitHub

Uh oh!

Accept AbstractSparseMatrixCSC for decompression - #298

Accept AbstractSparseMatrixCSC for decompression#298gdalle merged 10 commits intoJuliaDiff:mainJuliaDiff/SparseMatrixColorings.jl:mainfrom blegat:abstractsparseblegat/SparseMatrixColorings.jl:abstractsparseCopy head branch name to clipboard

Conversation

blegat commented Jan 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Jan 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

gdalle left a comment

Choose a reason for hiding this comment

Uh oh!

blegat commented Jan 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gdalle commented Jan 29, 2026

Uh oh!

blegat commented Jan 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gdalle commented Feb 18, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Mar 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

gdalle left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Accept AbstractSparseMatrixCSC for decompression#298
gdalle merged 10 commits into
JuliaDiff:mainfrom
blegat:abstractsparse

blegat commented Jan 19, 2026 •
edited
Loading

codecov Bot commented Jan 19, 2026 •
edited
Loading

blegat commented Jan 20, 2026 •
edited
Loading

blegat commented Jan 29, 2026 •
edited
Loading

codecov Bot commented Mar 18, 2026 •
edited
Loading