Repository navigation
perf: accelerate triangular rational solves - #278
Conversation
- Recognize upper and lower triangular systems through row permutations and solve by exact substitution with one reduction per component. - Preserve the general Bareiss path, canonical rational inputs, and typed singularity diagnostics without changing the public API. - Add permanent structured benchmarks and retain Rust 1.99 measurements: 11.15x faster prepared solves and 4.26x faster full adapters at D=8 on the reproduced triangular family. - Refresh the staged cc, syn, uv, Ruff, and Ty dependency/tooling pins. - Validation: just ci passes, including 1,002 Rust tests, 371 Python tests, doctests, examples, and benchmark compilation. Closes #246
acgetchell
added
performance
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to The optimization is limited to recognized triangular systems, and the recorded prerequisite chronology supports proceeding. No concrete merge-blocking risk remains. Pre-merge checks |
   |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Out of Scope Changes check |  |
pyproject.toml and uv.lock upgrade uv, Ruff, and ty. These tooling changes do not implement the #246 solver, benchmarks, tests, or performance study. The PR description also states that the upda… |
Remove the uv, Ruff, and ty upgrades, their lockfile changes, and the unrelated roadmap edits from this PR. Move them to separate maintenance or release-planning changes. Keep only documentation that supports the #246 solver, tests, bench… |
| Linked Issues check |  |
Issue #246 implementation requirements are supported. RationalMatrix::solve recognizes row-permuted triangular systems, performs exact substitution, and retains the Bareiss fallback. The new benchma… |
Provide reviewable evidence that #251 was complete before the optimization work began and that the regression was reproduced under the completed Rust 1.99 / LLVM 23 baseline before implementation. |
✅ Passed checks (2 passed)
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: improving performance for triangular rational solves.
Full details: Linked Issues check
-
Explanation
Issue
#246implementation requirements are supported.RationalMatrix::solverecognizes row-permuted triangular systems, performs exact substitution, and retains the Bareiss fallback. The new benchmarks separate construction, prepared solve, and adapter solve across the required structures and coefficient types. The regression tests cover exact residuals, independent Gaussian agreement, dimensions 0–8, permutations, wide rationals, mixed signs, zero RHS, fallback cases, and singularity metadata. The public API, rational validation, exact arithmetic, and shared Bareiss backend remain unchanged. The reported Rust 1.99 measurements support the required baseline and performance checks. The evidence does not establish that#251was complete before optimization work began, as required by#246.
Full details: Out of Scope Changes check
-
Explanation
pyproject.tomlanduv.lockupgrade uv, Ruff, andty. These tooling changes do not implement the#246solver, benchmarks, tests, or performance study. The PR description also states that the updates were not part of the timing runs.docs/roadmap.mdadds broad v0.5.0 release planning for unrelated issues, release tooling, publication, and const generics. Those edits do not document the#246implementation or its required evidence.Resolution
Remove the uv, Ruff, and
tyupgrades, their lockfile changes, and the unrelated roadmap edits from this PR. Move them to separate maintenance or release-planning changes. Keep only documentation that supports the#246solver, tests, benchmarks, or performance study.
- Fix all pre-merge checks with AI
✨ Finishing Touches
-
-
📝 Generate docstrings
-
-
-
-
-
- Commit to this branch
- Create a new PR
-
-
-
-
🧪 Generate unit tests (beta)
-
-
- Commit to this branch
- Create a new PR
-
-
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #278 +/- ##
==========================================
+ Coverage 98.12% 98.14% +0.01%
==========================================
Files 14 14
Lines 7112 7160 +48
==========================================
+ Hits 6979 7027 +48
Misses 133 133
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
- Plan the stable-Rust v0.5.0 release sequence and conditional algebraic floating-point adoption, with compatible follow-up work in v0.5.1. - Defer expanded const generics until the required Generic Const Arguments capabilities are stable. - Clarify that research-repo-tools is a contributor development dependency. - Describe dense benchmark controls as row-permuted while preserving measured inputs and retained performance evidence. Refs #123, #246, #250
1 participant
RationalMatrix::solvepreviously performed fraction-free elimination even when a row permutation made the system triangular. This adds structure recognition and direct exact substitution, reducing each solution component once. General and singular inputs retain the existing Bareiss path and typed singularity diagnostics. The public API, rational validation, and shared integer backend are unchanged.Permanent benchmarks cover diagonal, upper/lower triangular, row-permuted triangular, sparse, and dense systems through D=8 with dyadic and non-dyadic coefficients. Regressions check exact residuals and independent Gaussian agreement through D=0–8, including arbitrary permutations, wide rationals, mixed signs, zero RHS, and singular inputs.
On the reproduced row-permuted triangular family, comparable Rust 1.99 measurements on an Apple M4 Max give:
The full adapter beats the captured legacy Gaussian reference at every size 2–8, with non-overlapping marginal 95% intervals. The dense Hilbert solve is effectively unchanged; the dyadic dense control increases 0.73% for prepared solves and 1.77% for the full adapter. These are synthetic fixture results, with no whole-triangulation performance claim.
The performance study retains all measurements, raw samples, commands, source hashes, and limitations. Both timing phases used the same baseline lockfile. This PR also includes the staged
cc/synlockfile refresh and uv/Ruff/Ty updates; those later updates were not part of the timing runs.Validation:
just cipasses on the final dependency set, including 1,002 Rust tests, 371 Python tests, default/exact doctests, Clippy, documentation, examples, and benchmark compilation. Markdown and spelling checks pass; retained source and measurement hashes were verified.Closes #246
Summary by CodeRabbit