Repository navigation
perf(csr): one-pass n-ary merge, row gather for permuting reindex - #1014
Conversation
) Make CSRLinearExpression.added n-ary in one COO pass (as in #992), gather rows directly when reindexed only permutes cells, and index aggregated rows in the store's index dtype.
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 improve performance by 44.36%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | test_to_lp[nodal_balance_sparse-severity=0] |
3.9 MB | 2.5 MB | +54.8% |
| ⚡ | test_to_lp[nodal_balance_sparse-severity=100] |
3.9 MB | 2.5 MB | +54.71% |
| ⚡ | test_to_lp[nodal_balance_sparse-severity=50] |
3.9 MB | 2.5 MB | +54.71% |
| ⚡ | test_to_lp[knapsack-n=10000] |
2.1 MB | 1.4 MB | +52.91% |
| ⚡ | test_to_lp[storage-n=250] |
38.5 MB | 25.8 MB | +49.12% |
| ⚡ | test_to_lp[merge_balance-severity=0] |
2.7 MB | 1.8 MB | +48.43% |
| ⚡ | test_to_lp[rolling-severity=0] |
2 MB | 1.4 MB | +48.07% |
| ⚡ | test_to_lp[nodal_balance-severity=0] |
3.8 MB | 2.6 MB | +45.27% |
| ⚡ | test_to_lp[nodal_balance-severity=50] |
3.8 MB | 2.6 MB | +45.04% |
| ⚡ | test_to_lp[storage-n=10] |
1.6 MB | 1.1 MB | +44.89% |
| ⚡ | test_to_lp[piecewise-n=1000] |
1.9 MB | 1.4 MB | +37.3% |
| ⚡ | test_to_lp[basic-n=250] |
29.9 MB | 21.8 MB | +37.15% |
| ⚡ | test_to_lp[expression_arithmetic-n=250] |
46.6 MB | 35.3 MB | +32.01% |
| ⚡ | test_to_lp[sos-n=1000] |
1.4 MB | 1.1 MB | +21.19% |
Tip
Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.
Comparing perf/csr-kernel-1010 (d6da509) with master (7e007ff)2
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. ↩
-
No successful run was found on
master(69dd5c9) during the generation of this report, so 7e007ff was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩
Closes #1010. Part of #972 (phase 2, PR 7).
Note
The following content was generated by AI.
Changes proposed in this Pull Request
CSRLinearExpression.added(self, *others)now takes any number of operands on the same grid and builds the sum in a single COO pass._try_csr_mergecalls it once instead of folding pairwise. Explicit zero coefficients and absent cells keep their current behaviour. Auxiliary coordinates are united, earlier operands take precedence, and conflicting ones still raise.reindexedonly permutes the existing cells (no dropped and no new labels), it gathers the CSR rows and constants directly instead of going through COO.aggregated. The aggregated row indices are cast to the store's index dtype before the COO to CSR conversion.The
added(self, *others)change is byte-identical to the one in open PR #992, so the two PRs merge without conflict.Chained
+is unchanged, because+is binary.Benchmark (scratch script
dev-scripts/bench_1010.py, before → after)+Checklist
AGENTS.md).doc.doc/release_notes.rstof the upcoming release is included.