perf(expressions): sparse @/dot drops zero-coeff terms; vectorise densify_terms (#748) - #939
Merged
Merged
Conversation
Merging this PR will regress 1 benchmark
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing 
Footnotes
|
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. |
…e densify_terms (#748) LinearExpression.__matmul__ against a constant matrix that holds zeros densified the result to one term per contracted member. The bare @ now compacts the term dimension to the widest non-zero cell via densify_terms, under both legacy and v1 semantics; a zero-free constant is untouched. On PyPSA's Kirchhoff Voltage Law constraint this cuts 852 terms to 3. densify_terms is rewritten: it counted non-zero positions with a Python loop that scaled quadratically and allocated the output at the full term width. It is now vectorised, allocates only the compacted width, returns self when no zeros are present, works on QuadraticExpression, and no longer raises on expressions without coordinate dimensions.
…action zero operand (#748) Rename the __matmul__ local `is_constant` to `other_is_const` so it no longer shadows the imported `is_constant` helper and the `LinearExpression.is_constant` property, which carry the unrelated "no variables present" meaning. Add a regression test for `variable @ vector` that contracts away every coord dim with a zero-containing operand, the scalar-dot shape the other matmul tests did not exercise.
FabianHofmann
force-pushed
the
fix/748-sparse-matmul-densify
branch
from
September 7, 2026 07:55
5386ff9 to
da73f9b
Compare
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 #748.
The sparse
@/dotpath was blowing up the term dimension on PyPSA's KVL constraint, anddensify_termsitself was quadratic. This fixes both.Note
The following content was generated by AI.
Changes proposed in this Pull Request
Sparse
@/dotno longer densifies (#748).LinearExpression.__matmul__computed(self * other).sum(dim=common_dims), which stacks every contracted member into the term dimension, even where the constant matrix is zero. Against a sparse constant (mostly zeros) this produced one term per member. The matmul now compacts the zero-coefficient terms viadensify_termswhenever the constant operand contains zeros, under both legacy and v1 semantics. A zero-free constant is untouched, so dense matmuls carry no overhead.merge.densify_termsrewritten. The old implementation counted non-zero positions with a Python loop that scaled quadratically in the number of non-zero terms, and allocated the compacted output at the full original term width. It is now fully vectorised, allocates only the compacted width, and returns the expression unchanged when it holds no zeros.IndexErroron expressions without coordinate dimensions (expr.sum(drop_zeros=True)over all dimensions), which the new sparse@path made reachable via an all-zero constant.QuadraticExpression, where it previously indexed the_factoraxis as the term axis.Tests. New cases in
test/test_linear_expression.pycover: sparse operand compacts, dense operand untouched, all-zero operand yields the zero expression, in-order row compaction, no-op when zero-free, scalar/all-zerodrop_zeros, and the quadratic path.Checklist
AGENTS.md).doc.doc/release_notes.rstof the upcoming release is included.