I’ve completed a thorough trace of the B2 implementation against the approved design. I verified the kernel, the device API, the sweep integration, the non-CUDA guarding, stream serialization, fold isolation, and the test — and cross-checked the actual diff against HEAD. Here is the review.


Adversarial Review — B2 device-resident dual-Ridge (upload-once K + reuse-buffers Cholesky)

VERDICT: ship

The implementation faithfully matches the approved _b2_fable_design.md §3/§4. I found no correctness bugs, no fold leakage, no ABI/scope violation, and no non-CUDA-build breakage. The numerical-equivalence argument holds and the test asserts it correctly. Two non-blocking process/style notes below. (Per the brief, the orchestrator runs the GPU tests + bench separately to confirm the numbers — this review is static.)


Findings

1. [OK] add_scaled_identity is bit-identical to host K[i*n+i]+=λ. cuda_kernels.cu:193-202 — adds to A[i*n+i] only (layout-agnostic diagonal), plain fp64 += with no multiply so FMA contraction is impossible, i>=n guard correct. Launcher cuda_kernels.cu:311-329: n==0cudaSuccess, null/>INT_MAX guard, grid ceil(n/256), returns cudaGetLastError(). Diagonal index is identical in row/column-major, so the matrix fed to potrf is byte-for-byte the matrix spd_solve would upload after host-forming K+λI.

2. [OK] prepare_dual_ridge uploads K + column-major B0 exactly once and never throws. cuda_dispatch.cpp:1292-1361. Mirrors spd_solve’s guards (runtime/null/zero/int-range/mul_overflows), uploads full symmetric K (:1331), packs Bcm[target*n+i]=B[i*q+target] (:1334-1338), computes potrf_bufferSize once into h->lwork (:1344-1346) — content-independent so valid even though K has no λ yet. Every exception path (incl. bad_alloc) returns nullptr (:1349-1360). Caller treats nullptr as host fallback. Correct.

3. [OK] dual_ridge_solve re-copies into work buffers each call; pristine dB0 is never the potrs target. cuda_dispatch.cpp:1363-1449. copy_d2d(dK→ws.dA) (:1394), add_scaled_identity(ws.dA,λ) (:1396), copy_d2d(dB0→ws.dB) (:1398) — potrf/potrs operate on ws.dA/ws.dB, so dK/dB0 stay pristine for the next λ. Return contract matches spd_solve: potrf info >0→1, <0→2 (:1407-1416); potrs info ≠0→2 (:1423-1426); column-major→row-major unpack (:1430-1434); try/catch→2. The lwork<0 guard at :1376 correctly precedes ws.ensure’s static_cast<size_t>(lwork) at :1392.

4. [OK] No fold leakage; thread-local workspace is fully overwritten and shrink-safe. Each materialized fold owns its own PreparedDualRidge (own dK/dB0) via fold_prepared_dual[fold_ix].handle (sweep.cpp:2100-2105); there is no shared persistent K. The single thread_local ReusableDualWorkspace (cuda_dispatch.cpp:1388) has all leading /n·q elements rewritten by the two D2D copies every call, and ReusableDeviceBuffer::ensure only grows (:162-179) while potrf/potrs use ld=ni on the leading region — stale tail bytes are never read. dWork is sized per-call to max(seen lwork) and each potrf is passed its own fold’s handle->lwork, which is ≤ capacity regardless of fold order. Holds across separate run_moment_sweep/Context calls on the same thread too.

5. [OK] Numerical equivalence + the test are real and assert the right thing. Because the matrix into potrf is bit-identical to spd_solve’s and the same cuSOLVER runs on the same GPU, dual_ridge_solve reproduces spd_solve to ~1e-12. test_internal_linalg.cpp:372-462 builds an un-bumped SPD gram, and for λ∈{1e-3,0.1,1.0,10.0} compares dual_ridge_solve(h,λ) against spd_solve(K+λI) (the B1 path, not host QR) with max|Δ|≤1e-8 (:435-443); the singular diag(1,…,1,0) case asserts dual_ridge_solve(...,0.0)==1 (:447-459). GPU-self-skips (:373-376), registered at :476. Solid.

6. [OK] Stream serialization is correct — no race despite the switch to async D2D. spd_solve used synchronous copy_h2d; dual_ridge_solve uses async copy_d2d_contiguous (cudaMemcpyAsync, null stream — :528-531) + a null-stream kernel, then potrf on state().solver. state() (cuda_dispatch.cpp:48-86) never calls cusolverDnSetStream (confirmed: no occurrence in the file), so the solver is bound to the null stream. LocalCublasHandle (:559-570) is the only SetStream user and is a separate per-stream handle. All ops issue to the null stream from one serial thread → D2D → kernel → potrf are ordered; copy_d2h(&info) forces the sync. Matches design trap #4.

7. [OK] Non-CUDA build is correctly guarded — not compiling by accident. struct PreparedDualGpu; is forward-declared unconditionally (sweep.cpp:86), defined only under #if defined(N4M_USE_CUDA) (:88-114). The three consumers take const PreparedDualGpu* gpu = nullptr (:564, :609, :661) — a pointer to an incomplete type, which is legal; gpu is dereferenced (->valid(), ->handle) only inside the #if block of solve_dual_alpha (:542-553), with (void)gpu; in the #else. fold_prepared_dual (:2072) and the call-site dual_gpu selection (:2239-2244) are #if/#else-guarded (nullptr in non-CUDA). No sizeof/instantiation of the incomplete type in non-CUDA. Correct by construction, not by luck.

8. [OK] Gating, fallback, and N4M_CUDA_RIDGE_DISABLE all preserved. Prepare is gated on lambdas.size()>1U && ridge_cuda_dual_enabled(n) (sweep.cpp:2097-2098), reusing the n≥256 threshold + runtime check + disable-env (:465-478). solve_dual_alpha falls to the unchanged solve_dual_spd on nullptr handle, return 1, or return 2 (:543-556). With N4M_CUDA_RIDGE_DISABLE set, no handle is prepared and solve_dual_spd also takes the host-QR branch → byte-for-byte the pre-GPU host path.

9. [OK] Scope — only the 3 grid consumers rerouted; final fits default-null; no public/ABI surface. The diff (+720/−18) is purely additive except the 18 deletions, which are exactly the three inline K=design.K; K[i*n+i]+=λ; solve_square_qr(...) blocks replaced by solve_dual_alpha(...) (verified in git diff). Final single-λ fits at sweep.cpp:2625 and :2729 omit the arg → default nullptr → unchanged behavior. New symbols live in n4m::cuda_dispatch (internal header cpp/src/core/, not cpp/include/n4m/, no N4M_API, hidden visibility, not in the N4M_1 map) → not in nm -D; ABI stays 1.22.0. No catalog/Python/.def/.map change. (The orchestrator’s expected_symbols_*.txt diff is the authoritative confirmation.)

10. [STYLE] B1 and B2 are bundled in one uncommitted working tree — the “B1 already shipped” premise doesn’t match git. spd_solve, solve_dual_spd, and ridge_cuda_dual_enabled do not exist at HEAD (fcff454); they are added in the same uncommitted blob as B2. So “B1’s spd_solve/solve_dual_spd UNCHANGED” can’t be checked against a committed baseline — within this blob they’re present and B2’s additions don’t touch them, which is functionally what matters. One real consequence to flag for the bench gate: vs HEAD, the dual CV consumers switch from host QR to GPU Cholesky in this change (a B1 decision), so the host-vs-GPU ≤1e-8 parity run must pass for the whole dual path, not just the prepared variant — but B2’s dual_ridge_solve is bit-identical to spd_solve, so B2 adds zero new divergence on top of B1. Suggested fix: split into two commits (B1 per-call routing, then B2 prepared handle) so the parity delta is attributable; not a code defect.

11. [STYLE] Redundant GPU attempt on the not-PD fallback (harmless, cold path). When dual_ridge_solve returns 1, solve_dual_alpha falls to solve_dual_spd, which (since ridge_cuda_dual_enabled is still true) re-attempts spd_solve(K+λI) — that also returns 1 on the same matrix — before reaching host QR (sweep.cpp:554-556:486-498). One wasted GPU Cholesky, only on a non-PD K+λI (essentially never for λ>0 on a PSD gram). Optional: have the GPU fallback go straight to solve_square_qr when the prepared solve already reported not-PD. Not worth blocking on.


The core path is correct, the device/host equivalence is sound, the fallback chain is intact, and the non-CUDA build is guarded properly rather than accidentally. Ship after the orchestrator confirms the GPU equivalence test and the BERRY nsys/RMSEP parity bench.