From 6178f685590e439890ba0051d3a6b3ffa0251ed3 Mon Sep 17 00:00:00 2001 From: jgrusewski Date: Fri, 8 May 2026 20:57:21 +0200 Subject: [PATCH] fixup(sp17-pp.2): lock-test producer constants for symmetry Per code-quality review of a225926e5: ADVANTAGE_CLIP_SAFETY_FACTOR (1.5) and ADVANTAGE_CLIP_EMA_ALPHA (0.01) were declared `pub` but not asserted in the sp17_dueling_slot_layout_locked test. All other named constants in the SP17 ISV block are pinned by the lock test; this extends the same coverage to the two producer constants that the Phase 5 advantage_clip_bound producer kernel will consume. Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/ml/src/cuda_pipeline/sp14_isv_slots.rs | 3 +++ docs/dqn-wire-up-audit.md | 2 ++ 2 files changed, 5 insertions(+) diff --git a/crates/ml/src/cuda_pipeline/sp14_isv_slots.rs b/crates/ml/src/cuda_pipeline/sp14_isv_slots.rs index 0180f79f6..9ad87f45c 100644 --- a/crates/ml/src/cuda_pipeline/sp14_isv_slots.rs +++ b/crates/ml/src/cuda_pipeline/sp14_isv_slots.rs @@ -1042,6 +1042,9 @@ mod tests { // Category-1 dimensional safety bounds per `feedback_isv_for_adaptive_bounds`. assert_eq!(ADVANTAGE_CLIP_BOUND_MIN, 0.1); assert_eq!(ADVANTAGE_CLIP_BOUND_MAX, 100.0); + // Producer constants (consumed by Phase 5 advantage_clip_bound producer kernel). + assert_eq!(ADVANTAGE_CLIP_SAFETY_FACTOR, 1.5); + assert_eq!(ADVANTAGE_CLIP_EMA_ALPHA, 0.01); } #[test] diff --git a/docs/dqn-wire-up-audit.md b/docs/dqn-wire-up-audit.md index 05d7c9ccf..6c6bdf272 100644 --- a/docs/dqn-wire-up-audit.md +++ b/docs/dqn-wire-up-audit.md @@ -6,6 +6,8 @@ Allocates the 9 ISV slots that drive the SP17 dueling-Q identifiability diagnostic chain. **Additive infrastructure only — no consumer wiring in this commit.** The Phase 1 mean-centering producer (atomic across `compute_expected_q` + `c51_loss` + `c51_grad` + `mag_concat_qdir`) lands in the next 4 commits per `feedback_no_partial_refactor`. +**Fixup (post-quality-review):** `ADVANTAGE_CLIP_SAFETY_FACTOR` and `ADVANTAGE_CLIP_EMA_ALPHA` added to `sp17_dueling_slot_layout_locked` for assertion-coverage symmetry with the other named constants in the SP17 block. No code-path or contract change. + ### Why SP17 The codebase has separate V-head (`on_v_logits_buf`) and per-branch A-head (`on_b_logits_buf`) buffers, but combines them naively as `logit = v_row[z] + adv_a[z]` — no `A − mean_a A` subtraction. This is over-parameterization without identifiability: the optimizer is mathematically free to learn the degenerate `V ≈ Q(Flat), A ≈ Q(a) − Q(Flat)` decomposition, which IS the Q(Flat) attractor we have been fighting since SP11. The fix is small and surgical: insert mean-zero projection at 4 existing aggregation sites (Phase 1-3). PP.2 lays the ISV slot infrastructure for the diagnostics that observe whether the projection is doing real work.