Skip to content

perf(linalg): slim solve_ols marshalling - 15.3->7.8 GB rust-side footprint, -21% firm-panel fits, outputs preserved#607

Merged
igerber merged 1 commit into
mainfrom
perf/solve-ols-marshalling
Jul 3, 2026
Merged

perf(linalg): slim solve_ols marshalling - 15.3->7.8 GB rust-side footprint, -21% firm-panel fits, outputs preserved#607
igerber merged 1 commit into
mainfrom
perf/solve-ols-marshalling

Conversation

@igerber

@igerber igerber commented Jul 3, 2026

Copy link
Copy Markdown
Owner

Summary

  • Slims the solve_ols hot path on both backends - the memory census from the solver-phase diagnosis, implemented. Rust (rust/src/linalg.rs): norms from the borrowed view, equilibration fused into the single owned faer copy (replacing a defensive copy + scaled clone + conversion copy), U'y computed directly off the faer SVD factor via deterministic column-order accumulation (the old path materialized a full ndarray copy of U to compute one k-vector), and factors dropped before the fitted/residual/vcov stage so thin-SVD U never coexists with the cluster-vcov scores block. Python (diff_diff/linalg.py): _detect_rank_deficiency uses qr(mode="r") - same dgeqp3 R/pivot, skips forming the discarded Q (sibling fixed in _rank_guarded_inv); _equilibrated_lstsq materializes its scaled temporary F-order and passes overwrite_a=True so gelsd consumes the copy we already own (scipy silently ignores overwrite_a for C-order input - the F-order materialization is what makes it real).
  • Measured, allocation-level: rust-side allocator high-water on a 2.4M x 130 clustered solve falls 15.32 GB -> 7.81 GB (6.14 -> 3.13 n x k blocks), matching the code census exactly. Instrumented via a new feature-gated counting allocator (--features alloc-profile, never compiled into wheels) because resident-set metrics cannot verify this class of fix on macOS: the compressor evicts exactly the cold wasted copies, and an interleaved A/B showed the historical "13.4 vs 21 GB backend RSS gap" was a run-order artifact, not a backend property (full corrected diagnosis in docs/performance-plan.md).
  • The removed copies also cost time - frozen-code benchmark arms (3 repeats, strictly sequential) measured CV-clear improvements with no scenario regressing on either backend: firm-panel rust fits 25.84 -> 20.34 s (-21%), county event studies -12% rust / -13% python (the skipped dorgqr - rank-QR was ~20% of that fit), geo rust -6%, survey rust -2%. Committed after/rust baselines updated with identical estimates + fresh timings; findings tables regenerated (headline arc on the firm-panel workload now 92.96 -> 20.34 s, 4.6x, since the initiative baseline).
  • Outputs preserved, proven: the python-arm identity gate printed deltas of exactly 0.0 on every scenario (the mode="r"/overwrite_a changes are bit-identical by construction - same LAPACK factorizations on the same values - and now locked by tests); the rust arm sits at the usual 1e-13..1e-16 cross-backend level. TODO's solver-memory row is resolved into a narrow Parked floor note (remaining residency is inherent to SVD-based lstsq; further reduction = the tall-skinny-QR change parked with the QR-reuse row), and the stale ndarray-to-faer conversion row is removed (its cited site no longer exists).

Methodology references (required if estimator / math changes)

  • Method name(s): OLS via column-equilibrated truncated SVD with R-style pivoted-QR rank detection - marshalling/allocation changes only; no estimand, equilibration-semantics, rank-decision, or inference change. Rank decisions are bit-identical (mode="r" returns the same dgeqp3 R and pivot); python solve outputs are bit-identical; rust outputs move only at the documented cross-backend parity tolerances.
  • Paper / source link(s): docs/methodology/REGISTRY.md solve_ols scale-invariance Note (extended with an explicit sentence: the Python gelsd and Rust faer thin-SVD backends implement the same contract and agree at parity-suite tolerances, never bit-identically).
  • Any intentional deviations from the source (and why): None.

Validation

  • Tests added/updated: tests/test_linalg.py - cross-mode QR lock (mode="r" vs mode="economic": identical R diagonal + pivot on collinear and scale-artifact fixtures; rank/dropped hard-coded, pivots locked by same-session equality since dgeqp3 tie-breaks are BLAS-dependent) and an F-order/overwrite_a bit-equality lock (like-for-like reference, caller's X proven unmutated). tests/test_rust_backend.py - underdetermined n < k parity through the slimmed raw-kernel path (residual/exact-fit contract; the public solve_ols rejects n < k). Existing rust-vs-numpy parity suites and cargo tests pass unchanged (350 rust-mode + 480 python-mode sweep, 45 cargo tests).
  • Backtest / simulation / notebook evidence (if applicable): frozen-code FE-absorption benchmark arms with identity gates (python deltas exactly 0.0; rust 1e-13..1e-16; tail_stress exempt deltas 0.0); allocator before/after quoted above; committed baselines + auto-generated findings tables updated in docs/performance-plan.md.

Security / privacy

  • Confirm no secrets/PII in this PR: Yes

Generated with Claude Code

….8 GB rust-side high-water, CV-clear speed wins, outputs preserved

Closes the solver-memory investigation with the census fix, an
allocation-level measurement instrument, and the corrected diagnosis on
record:

- rust/src/linalg.rs::solve_ols: column norms computed from the borrowed
  numpy view (bit-identical scan order), equilibration fused into the
  SINGLE owned faer copy (replaces defensive copy + scaled clone +
  conversion copy), U'y computed directly off the faer SVD factor via
  deterministic column-order accumulation (the old path materialized a
  full ndarray copy of U for one k-vector dot), and the factorization +
  input dropped before the fitted/residual/vcov stage so thin-SVD U never
  coexists with the cluster-vcov scores block. Shared ndarray_to_faer
  helper untouched (trop.rs still uses it).
- diff_diff/linalg.py: _detect_rank_deficiency uses qr(mode="r") - same
  dgeqp3 R and pivot as mode="economic" (locked by a cross-mode
  equality test), skipping the discarded-Q formation (dorgqr) that cost
  ~20% of county-class fits; sibling in _rank_guarded_inv.
  _equilibrated_lstsq materializes its scaled temporary F-ORDER and
  passes overwrite_a=True (scipy honors overwrite only for F-contiguous
  input - a C-order temp would be a silent no-op), letting gelsd consume
  the copy we already own; bit-equality locked by test.
- rust/Cargo.toml + rust/src/alloc_profile.rs: feature-gated counting
  global allocator (--features alloc-profile, never in wheels) exposing
  reset_alloc_high_water / alloc_high_water_bytes for allocation-level
  before/after claims - resident-set metrics are unreliable for this on
  macOS (the compressor evicts cold pages; the historical "13.4 vs 21 GB
  backend gap" was a run-order artifact - interleaved A/B measured no
  systematic backend RSS difference).

Measured (2.4M x 130 clustered solve, firm_churn-class):
- rust-side allocator high-water 15.32 GB (6.14 n x k blocks) -> 7.81 GB
  (3.13 blocks); remaining floor = fused input copy + thin-SVD U
  transient + vcov scores, matching the code census exactly.
- Frozen-code benchmark arms (3 repeats, identity-gated vs committed
  baselines): python-arm estimates BIT-IDENTICAL (deltas exactly 0.0 on
  every scenario); rust-arm at the usual 1e-13..1e-16. No scenario
  regresses; CV-clear improvements: firm rust 25.84 -> 20.34 s (-21%),
  county 1.76 -> 1.55 s rust / 2.39 -> 2.09 s python (skipped dorgqr),
  geo rust -6%, survey rust -2%. Committed after/rust baselines updated
  (identical estimates, fresh timings) + findings tables regenerated.

Tests: cross-mode QR lock + F-order/overwrite bit-equality lock
(test_linalg.py), underdetermined n<k raw-kernel parity
(test_rust_backend.py), existing rust-vs-numpy parity suites unchanged.
Sweeps: 350 rust-mode + 480 python-mode green; 45 cargo tests; clippy
clean on touched files. TODO: solver-memory row resolved -> narrow Parked
floor note; stale ndarray-to-faer row removed; REGISTRY solve_ols Note
gains the explicit cross-backend tolerance sentence; full diagnosis in
docs/performance-plan.md.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown

Overall Assessment

✅ Looks good — no unmitigated P0/P1 findings.

Executive Summary

  • Affected methods: shared solve_ols OLS/rank handling and Rust raw OLS kernel; _rank_guarded_inv pivot selection path also changes QR mode.
  • No undocumented methodology mismatch found against docs/methodology/REGISTRY.md.
  • The backend non-bit-identity / tolerance contract is documented in docs/methodology/REGISTRY.md:L546-L550.
  • The remaining solver memory floor is tracked in TODO.md:L124-L126, so it is not blocking.
  • I could not run tests locally because the review environment lacks NumPy; this review is static.

Methodology

  • Severity: P3 informational
    Impact: The PR changes numerical marshalling and accumulation order, but preserves the documented column-equilibrated least-squares and rank-detection contract. The registry explicitly documents Python gelsd vs Rust faer thin-SVD parity tolerances and non-bit-identical backend behavior at docs/methodology/REGISTRY.md:L546-L550.
    Concrete fix: None required.

Code Quality

  • Severity: None
    Impact: The Python changes are scoped to QR output usage and F-order lstsq marshalling at diff_diff/linalg.py:L145-L155 and diff_diff/linalg.py:L312-L328. The Rust kernel keeps the same solve stages while dropping large temporaries earlier at rust/src/linalg.rs:L80-L148.
    Concrete fix: None.

Performance

  • Severity: None
    Impact: The PR removes avoidable copies in the solver hot path and adds a feature-gated allocation profiler. The allocator is behind alloc-profile and not enabled by default.
    Concrete fix: None.

Maintainability

  • Severity: None
    Impact: The added comments explain why qr(mode="r"), F-order overwrite_a, and direct U'y accumulation are safe. No new public API or parameter propagation issue found.
    Concrete fix: None.

Tech Debt

  • Severity: P3 informational
    Impact: Remaining solver memory floor is explicitly tracked as parked/deferred in TODO.md:L124-L126, so it is accepted deferred work under the review policy.
    Concrete fix: None required.

Security

  • Severity: None
    Impact: No secrets, unsafe data handling, or user-controlled execution path found. The new profiling functions expose only allocation counters and are compiled only with --features alloc-profile at rust/src/alloc_profile.rs:L1-L10.
    Concrete fix: None.

Documentation/Tests

  • Severity: P3 informational
    Impact: The PR adds targeted tests for QR mode equivalence, F-order lstsq, and Rust underdetermined raw-kernel behavior at tests/test_linalg.py:L1602-L1660 and tests/test_rust_backend.py:L542-L579. I could not execute them locally because NumPy is not installed in this environment.
    Concrete fix: Ensure CI runs the added Python tests and Rust backend test suite.

@igerber igerber added the ready-for-ci Triggers CI test workflows label Jul 3, 2026
@igerber igerber merged commit abf21bd into main Jul 3, 2026
39 of 40 checks passed
@igerber igerber deleted the perf/solve-ols-marshalling branch July 3, 2026 19:04
@igerber igerber mentioned this pull request Jul 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-ci Triggers CI test workflows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant