feat(expressions): sparse @/dot for CSR-backed expressions - #961
Merged
Merged
Conversation
Contract `expr @ C` as one sparse matrix product instead of the dense broadcast intermediate, so peak memory scales with nnz(C) x nterm. CSR backing survives `@`; the result is the compact canonical form.
Build cost — v1 vs legacyv1 build peak & time relative to legacy, on this commit — not a comparison against master (that is CodSpeed).
Full table (time + peak, mean)📊 Interactive plots + CSV: download the semantics-report-v1-vs-legacy artifact from this run. Report-only · not a gate · refreshed on every push · obsolete once legacy is dropped. |
Merging this PR will regress 1 benchmark
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ❌ | test_to_lp[storage-n=250] |
30.8 MB | 38 MB | -18.88% |
| ⚡ | test_to_lp[sparse_network-n=10] |
1,343.2 KB | 718.3 KB | +86.99% |
| ⚡ | test_to_lp[expression_arithmetic-n=10] |
1,339.2 KB | 718.2 KB | +86.46% |
| ⚡ | test_to_lp[qp-n=1000] |
2.6 MB | 2 MB | +32.14% |
| ⚡ | test_to_lp[nodal_balance_sparse-severity=100] |
3.7 MB | 3.1 MB | +19.83% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing feat/sparse-matmul (fb783a9) with master (b0841a1)
Footnotes
-
181 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
`dot`/`@` accept DataArrays on both the sparse and dense paths, but the annotation still said `ndarray`. Also guard the `_csr` access in the aux-coords test.
This was referenced Sep 23, 2026
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
1 participant
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.
Closes #756.
Note
The following content was generated by AI.
Changes proposed in this Pull Request
Under v1 semantics,
expr @ C/expr.dot(C)against a constant now runs as one sparse matrix product instead of(self * other).sum(dim). The old path materialised acells(self) x cells(other's kept dims) x ntermbroadcast before a single zero was dropped, so peak memory scaled with the intermediate rather than with the result.CSRLinearExpression.contracted(matrix, contracted_dims, new_indexes)inlinopy/csr.pyevaluates(I_kept ⊗ Cᵀ) @ Ain chunks of 64 kept cells, so the operator cost isO(chunk · nnz(C))and independent of the number of kept cells. A single unchunked Kronecker product was measured at ~3.7x the result size in peak memory and was rejected, as was a COO-join formulation.LinearExpression._sparse_matmulruns before the operand conversion touchesself.coords, which has no CSR branch and would densify. A CSR-backed expression (fromgroupby(...).sum(sparse=True)) therefore stays CSR-backed through@, and(g @ C) + (h @ C)still merges sparsely._matmul_operand_to_matrixinlinopy/alignment.pyreuses the primitives_apply_constant_op_v1uses, so the §5 (user NaN), §8 (label mismatch / reorder) and §11 (auxiliary-coordinate conflict) errors are byte-identical to those of*. A test asserts that equality directly against the sparse path.QuadraticExpressionare untouched.Term layout
The sparse branch returns the compact canonical form: duplicate variables summed, terms ordered by variable label, explicit zeros pruned, padded to the widest cell. The dense path keeps one term per contracted member when
Cholds no zeros, sontermmay differ between the two; the values agree. v1 does not promise term layout. Cell activeness is carried byconstalone, unlikeCSRLinearExpression.added, which preserves explicit zeros through COO. See #925.Measured on master, before this change
The sparse case peaked higher than the fully dense one of similar output size, because the intermediate is
snapshot x branch x cycle x nterm, not the result. Atracemallocguard on the 1000x300x100 case now asserts a peak below a quarter of the dense rectangle; it measures 28.4 MB against a 120 MB bound.Review
The change was reviewed by four independent reviewer agents plus a verification pass. They found three crashes on degenerate shapes, where the sparse path raised but the dense path returned a result: a zero-length kept dimension, a zero-length contracted dimension, and a dimensionless expression. All three are fixed by bailing to the dense path, and each has a regression test that fails with its guard reverted. The reviewers also showed, by stubbing
_sparse_matmulto returnNone, that the first version of the dense/CSR test parametrisation passed with the feature disabled; the tests now assert which path ran, and 8 of them fail under that stub.Checklist
AGENTS.md).doc.doc/release_notes.rstof the upcoming release is included.