perf(#2731): cache-efficient matmuls in add_sae_separation_barrier — the 86% wall - #2771
Open
HomunculusLabs wants to merge 3 commits into
Open
perf(#2731): cache-efficient matmuls in add_sae_separation_barrier — the 86% wall#2771HomunculusLabs wants to merge 3 commits into
HomunculusLabs wants to merge 3 commits into
Conversation
…paration_barrier with cache-efficient .dot() The separation barrier's per-edge inner loop (penalties.rs:1418-1474) computed five matrix products with triple-nested scalar loops over p: cross = B_j·B_kᵀ, mb = cross·B_k, sjb = S_j·B_j, mtb = crossᵀ·B_j, skb = S_k·B_k At p=2048 the strided column access pattern (fixed o, varying row) thrashes the L1 cache. The SauersML#2731 profiling found add_sae_separation_barrier consumes 86.37% of the censoring cell at (p=2048, charts=32), running at 1.03 of 16 cores — a serial wall. Replaced with ndarray .dot() which uses the matrixmultiply crate's blocked traversal. The math is identical (same operations, different summation order from cache blocking). All 8 separation_barrier tests pass at the 1e-12 golden tolerance, including the dense-vs-deferred bit-equivalence test (separation_barrier_deferred_curvature_matches_dense_hbb_1610).
HomunculusLabs
marked this pull request as draft
August 13, 2026 16:08
…per review Engineer (gpt-5.6-sol) + oracle review found: - The 1e-12 'bit-for-bit equivalence' claim was under-tested: the golden test compares dense vs deferred, both using the new dot() implementation - The source comment said 'three' products instead of 'five' - The cross product was row-contiguous, not strided; only mb/mtb/sjb/skb had strided column access Changes: - Comment: 'identical' -> 'algebraically equivalent ... not bit-for-bit identical' - Comment: 'three' -> 'five', fix strided vs row-contiguous accuracy - PR body: same softening applied
Contributor
Author
|
Re-verified at current main ( |
HomunculusLabs
marked this pull request as ready for review
August 14, 2026 16:10
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
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.
What
The #2731 profiling found
add_sae_separation_barrierconsumes 86.37% of all user cycles in the censoring cell (p=2048, charts=32), running at 1.03 of 16 cores — a single-threaded serial wall that blocks the #2283 production shape.This PR replaces five hand-rolled scalar triple-nested matrix products with cache-efficient
ndarray.dot()calls (backed by thematrixmultiplycrate's blocked traversal).The problem
add_sae_separation_barrier(penalties.rs:1418-1474) computes per-edge decoder derivatives for the Jeffreys separation barrier. Each edge computes five matrix products as scalar loops over the ambient dimensionp:The inner loops for
mb,mtb,sjb, andskbaccessB_k[[b, o]]withbas the fast index andofixed — a strided column access pattern that thrashes L1 atp=2048. Each(a, o)entry ofmbwalkscross[a, :] · B_k[:, o], re-reading the columnoofB_kfor every rowa.The fix
Replace the five hand-rolled loops with
.dot():ndarray.dot()dispatches tomatrixmultiplywhich uses cache-blocked (MC × KC × NC) traversal. The math is identical — same operations, different summation order from blocking. Net: −28 lines, +24 lines.Why this is safe
The replacement is algebraically equivalent to the original scalar loops. The orientation of all five products was verified against the source:
bj.dot(&bk.t())→ B_j B_kᵀ (m_j, m_k)cross.dot(bk)→ M B_k (m_j, p)s_j.dot(bj)→ (B_j B_jᵀ) B_j (m_j, p)cross.t().dot(bj)→ Mᵀ B_j (m_k, p)s_k.dot(bk)→ (B_k B_kᵀ) B_k (m_k, p)The summation order differs (blocked vs sequential), so results are within existing numerical tolerances but are not bit-for-bit identical. The golden test
separation_barrier_deferred_curvature_matches_dense_hbb_1610passes because both the dense and deferred paths use the new implementation — it verifies internal consistency, not old-vs-new equivalence.All 8
separation_barrier*tests pass:Review
Reviewed through the RepoPrompt pipeline (engineer agent + oracle review):
What this does NOT do
This is one of two walls identified in #2731:
eighinfit_diagnostics_reportThey are sequential: fixing the barrier alone delivers the corner to the diagnostics wall. Both need fixing for production scale.
Expected speedup
At the reproducer scale (
p=2048, m=3, top_k=2): the five matmuls per edge have dimensions(3×3),(3×p),(3×p),(3×p),(3×p). The hand-rolled loops do3·3·2048 + 3·2048·3 + 3·3·2048 + 3·2048·3 + 3·3·2048 ≈ 100Kscalar multiply-adds per edge with strided access. Blocked traversal should give a meaningful wall-clock improvement but I have not measured it on the reproducer — my machine (M2 Ultra) is not the profiled node.The deeper win is at larger
m: when basis_size grows (e.g.m=8on richer manifolds), the(m×m)·pproducts become8·8·2048 = 131Kper product, and the strided access penalty scales withm.Scope
Single function, 52 lines changed, no API change, no new dependency. The
BTreeMapcarrier aggregation at line 1537 is a separate bottleneck — noted in the artifact but not touched here.