Skip to content

perf: characterize rational row-clearing allocation costs - #237

Merged
acgetchell merged 1 commit into
mainfrom
feat/233-row-clearing
Sep 5, 2026
Merged

perf: characterize rational row-clearing allocation costs#237
acgetchell merged 1 commit into
mainfrom
feat/233-row-clearing

Conversation

@acgetchell

@acgetchell acgetchell commented Sep 5, 2026

Copy link
Copy Markdown
Owner
  • Add small and wide-component rational benchmark fixtures across D=2–8.
  • Isolate component cloning and integer scaling costs from complete determinant, sign, and solve operations.
  • Publish allocation and timing data with provenance and reproduction steps supporting the existing borrowed implementation.

Closes #233

Summary by CodeRabbit

  • Performance

    • Added expanded rational-arithmetic benchmarks covering small and wide inputs.
    • Added allocation-focused measurements for determinant, solving, and row operations.
    • Added validation to confirm benchmark results remain mathematically correct.
  • Documentation

    • Documented the scope and interpretation of wide rational-input benchmarks.
    • Added a performance study comparing allocation behavior and timing across representative workloads.
    • Clarified which benchmark groups contribute to release performance reports.

- Add small and wide-component rational benchmark fixtures across D=2–8.
- Isolate component cloning and integer scaling costs from complete determinant, sign, and solve operations.
- Publish allocation and timing data with provenance and reproduction steps supporting the existing borrowed implementation.

Closes #233
@acgetchell acgetchell self-assigned this Sep 5, 2026
@acgetchell
acgetchell enabled auto-merge September 5, 2026 02:44
@coderabbitai

coderabbitai Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Essentials

Run ID: d2635cfb-56db-45a2-8720-03074aa6c6a1

📥 Commits

Reviewing files that changed from the base of the PR and between 25aa4c0 and 2688964.

⛔ Files ignored due to path filters (4)
  • Cargo.lock is excluded by !**/*.lock
  • docs/assets/rational-row-clearing-allocations.csv is excluded by !**/*.csv
  • docs/assets/rational-row-clearing-timings.csv is excluded by !**/*.csv
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (8)
  • Cargo.toml
  • benches/common/rational.rs
  • benches/exact.rs
  • docs/BENCHMARKING.md
  • docs/performance/rational-row-clearing.md
  • justfile
  • pyproject.toml
  • tests/rational_allocations.rs

Included review availability: 2 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 3 reviews per hour.


📝 Walkthrough

Walkthrough

The change adds shared validated rational fixtures for small and wide inputs, expands exact benchmarks across input families and dimensions, adds allocation-evidence tests, updates development-tool pins, and documents benchmark scope and row-clearing measurements.

Changes

Rational performance evidence

Layer / File(s) Summary
Validated rational fixtures and Gaussian references
benches/common/rational.rs
Adds deterministic rational input families, validated matrix and vector accessors, exact matrix-vector multiplication, and pivoting Gaussian determinant and solve references.
Expanded exact benchmark coverage
benches/exact.rs
Moves shared helpers into rational_bench, uses validated fixture accessors, and runs rational benchmarks for all input families across dimensions 2–8.
Allocation measurement tests and tooling
tests/rational_allocations.rs, Cargo.toml, justfile, pyproject.toml
Adds feature-gated allocation measurements for rational operations and updates the allocation-counter, rumdl, and semgrep development pins.
Benchmark scope and study record
docs/BENCHMARKING.md, docs/performance/rational-row-clearing.md
Documents canonical and diagnostic benchmark groups and records allocation, timing, reproduction, environment, and validation results.

Estimated code review effort: 4 (Complex) | ~45 minutes

Merge Risk: ⚪ Minimal · up to 26889

The benchmark fixtures, allocation measurements, and supporting tooling changes do not leave a concrete unresolved merge risk.

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The benchmark fixtures, allocation test, documentation, and allocation-counter dependency support issue #233. The rumdl update in justfile and the semgrep update in pyproject.toml are tooling-… Remove the unrelated rumdl and semgrep pin updates, or provide a linked requirement that explains why those updates are necessary for this pull request.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main change: characterizing allocation costs in rational row clearing.
Linked Issues check ✅ Passed The changes address issue #233 by adding D=2–8 rational benchmarks, representative and wide inputs, independent exact-result validation, allocation measurements, timing evidence, and reproduction docu…
Full details: Out of Scope Changes check

Explanation

The benchmark fixtures, allocation test, documentation, and allocation-counter dependency support issue #233. The rumdl update in justfile and the semgrep update in pyproject.toml are tooling-pin changes with no stated connection to the issue objectives.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/233-row-clearing

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Sep 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.00%. Comparing base (25aa4c0) to head (2688964).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #237   +/-   ##
=======================================
  Coverage   98.00%   98.00%           
=======================================
  Files          13       13           
  Lines        6579     6579           
=======================================
  Hits         6448     6448           
  Misses        131      131           
Flag Coverage Δ
unittests 98.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@acgetchell
acgetchell merged commit 188c35b into main Sep 5, 2026
20 of 21 checks passed
@acgetchell
acgetchell deleted the feat/233-row-clearing branch September 5, 2026 03:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

perf: Measure BigInt cloning in rational row clearing

1 participant