docs(sp5): close two minor review findings before A5

Two minor code-review nits accumulated across A1-A4 reviews; closing
them in a single docs/comment-only commit before Layer A continues.

1. tests/sp5_producer_unit_tests.rs module docstring (A4 review):
   the file header still claimed it covered only A1's two kernels
   even though A2/A3/A4 had each appended their tests. Updated header
   to enumerate all 9 tests across A1-A4 (+ 1 grad_cosine_sim test
   landed in 4da6b34d7) so a reader cold to the file can locate each
   producer's coverage without scanning the body.

2. gpu_dqn_trainer.rs:2854 stale field comment `[B, TOTAL_ACTIONS(11)]`
   (A1 review): comment was left at 11 since the pre-4-direction
   layout but the actual slot count is 13 (4 dir + 3 mag + 3 ord + 3
   urg). Updated to `[B, TOTAL_ACTIONS(13)]` with the per-branch
   breakdown inline so the field declaration matches the SP5 producer
   docstrings (e.g. q_branch_stats_kernel comments at line 277, 282).

Comment-only changes; no behavior change. cargo check + cargo test
--no-run both clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
jgrusewski
2026-05-01 22:35:18 +02:00
parent 4da6b34d78
commit d4d12abab2
3 changed files with 20 additions and 6 deletions

View File

@@ -2851,7 +2851,7 @@ pub struct GpuDqnTrainer {
mse_loss_dev_ptr: u64,
// ── Forward-only Q-value output ─────────────────────────────────
q_out_buf: CudaSlice<f32>, // [B, TOTAL_ACTIONS(11)]
q_out_buf: CudaSlice<f32>, // [B, TOTAL_ACTIONS(13)] — 4 direction + 3 magnitude + 3 order + 3 urgency
/// Snapshot buffers for td_errors + per_sample_loss during eval replay.
/// Prevents graph_forward replay from corrupting PER priorities.
eval_td_snapshot: CudaSlice<f32>,

View File

@@ -1,12 +1,24 @@
#![allow(unsafe_code)] // CUDA kernel launch + mapped-pinned memory.
//! SP5 Task A1 producer kernel unit tests.
//! SP5 Layer A producer kernel unit tests.
//!
//! Tests cover the two SP5 Task A1 CUDA kernels:
//! 1. `q_branch_stats_update`: per-branch Q-statistics reduction.
//! 2. `pearl_1_atom_update`: Pearl 1 atom-span headroom controller.
//! Tests cover the SP5 Layer A CUDA kernels across Tasks A1A4:
//!
//! Both tests use analytically-known synthetic inputs with no CPU reference
//! Task A1 (Pearl 1 atom-span + Q-stats source):
//! 1. `sp5_q_branch_stats_uniform_input_matches_analytical_ground_truth`
//! 2. `pearl_1_atom_update_v_half_tracks_max_abs_dev_with_bootstrap_headroom`
//! Task A2 (Pearl 3 NoisyNet σ):
//! 3. `pearl_3_sigma_bootstrap_at_target_entropy`
//! 4. `pearl_3_sigma_fraction_increases_when_entropy_below_target`
//! Task A3 (Pearl 2 loss budget):
//! 5. `pearl_2_budget_flat_regime_iqn_floor_c51_takes_remainder`
//! 6. `pearl_2_budget_sharp_regime_iqn_dominates`
//! Task A4 (Pearl 4 per-group Adam β/β/ε + auxiliary cosine-sim):
//! 7. `pearl_4_adam_hparams_high_stability_yields_long_memory`
//! 8. `pearl_4_adam_hparams_low_stability_yields_short_memory`
//! 9. `grad_cosine_sim_per_group_dot_norm_and_writeback`
//!
//! All tests use analytically-known synthetic inputs with no CPU reference
//! implementation, per `feedback_no_cpu_test_fallbacks.md`. All mapped-pinned
//! memory (no DtoH), all `#[ignore]`-gated for GPU.
//!

View File

@@ -2,6 +2,8 @@
**Status:** Populated during Plan 1 Task 6 (A.5 orphan audit). Updated on every commit per Invariant 7.
SP5 docs/comment-only fix-up — close two minor review findings before Layer A Task A5 (2026-05-01): two minor code-review nits accumulated across A1-A4 reviews; closed in a single docs/comment-only commit. (1) `tests/sp5_producer_unit_tests.rs` module docstring (A4 review) updated to enumerate all 9 SP5 Layer A tests (q_branch_stats + pearl_1 + pearl_3 × 2 + pearl_2 × 2 + pearl_4 × 2 + grad_cosine_sim) so a reader cold to the file can locate each producer's coverage without scanning the body. (2) `gpu_dqn_trainer.rs:2854` stale field comment `// [B, TOTAL_ACTIONS(11)]` (A1 review) updated to `// [B, TOTAL_ACTIONS(13)] — 4 direction + 3 magnitude + 3 order + 3 urgency` so the field declaration matches the SP5 producer docstrings (e.g. q_branch_stats_kernel comments at line 277/282) and the 4-branch action layout established in commit `2fb30f098`. Comment-only changes; no behavior change. cargo check + cargo test --no-run both clean (11 pre-existing warnings, none new).
SP5 Task A4 fix-up — grad_cosine_sim_update unit test added (2026-05-01): code-quality review caught that `pearl_4_adam_hparams_kernel` had direct GPU tests but the auxiliary `grad_cosine_sim_kernel` had none. Test 7 and Test 8 launched the hparams kernel directly with pre-baked cosine inputs, never exercising the per-group reduction (dot + 2× L2 norm sums) or the writeback (grad_curr → grad_prev for next step's comparison). The writeback in particular is load-bearing for cross-step cosine evolution — if broken, every subsequent step's cosine_sim is computed against a stale or mis-aligned previous gradient. Fix adds Test 9 `grad_cosine_sim_per_group_dot_norm_and_writeback` with 8 analytically-known synthetic group cases: identical (cos=+1), orthogonal (0), antiparallel (-1), Pythagorean (cos=+1, |c|=5), cold-start curr (zero grad → cos=0, |c|=0), cold-start prev (Pearl A sentinel state → cos=0, |c|=1), and 2 repeats. NO CPU oracle — expected values are unit-vector cosines from the kernel formula. Writeback verified by reading grad_prev_buf after the kernel runs and asserting bit-identity with grad_curr_buf for all 32 elements. Touched: `crates/ml/tests/sp5_producer_unit_tests.rs` (+1 cubin const + 1 loader + 120 LOC test). cargo test --no-run clean.
SP5 Task A4 — Pearl 4 per-group Adam β1/β2/ε GPU producers (Layer A, 2026-05-01): two new CUDA kernels (`grad_cosine_sim_kernel.cu`, `pearl_4_adam_hparams_kernel.cu`) and one `MappedF32Buffer` (`grad_prev_buf_per_group`) land as Layer A additive producers that feed ISV slots [226..250) with per-param-group Adam hyperparameters. `grad_cosine_sim_update`: single-block 8-thread kernel (one thread per SP4 param group: DqnTrunk/Value/Branches/IQN/IqlHigh/IqlLow/Attn/Curiosity), reads `grad_buf[total_params]` + `grad_prev_buf[total_params]` + `group_param_offsets[9]` (prefix sums in element units); computes `dot = Σ gc·gp`, `norm_curr_sq = Σ gc²`, `norm_prev_sq = Σ gp²` per group; writes `cos_sim[8]→scratch[131..139)` and `l2_norm[8]→scratch[139..147)`; writebacks `grad_curr → grad_prev_buf` for the next step's comparison (same thread — no race). `pearl_4_adam_hparams_update`: single-block 8-thread kernel, reads `cos_sim[8]` + `l2_norm[8]` from scratch; clips stability to `[0, 1]`; computes `β1 = 0.85 + 0.10×stability ∈ [0.85, 0.95]`, `β2 = 0.99 + 0.0095×stability ∈ [0.99, 0.9995]`, `ε = clamp(grad_norm × 1e-7, 1e-10, 1e-6)`; writes `β1[8]→scratch[147..155)`, `β2[8]→scratch[155..163)`, `ε[8]→scratch[163..171)`. Structural envelopes (Invariant 1 anchors per spec): β1∈[0.85, 0.95], β2∈[0.99, 0.9995], ε∈[1e-10, 1e-6]. `launch_sp5_pearl_4_adam_hparams` fires 24 `launch_apply_pearls` calls: β1[8] → ISV[226..234), β2[8] → ISV[234..242), ε[8] → ISV[242..250). Uses `ALPHA_META_PEARL_4=5e-4` (half SP4 default 1e-3) per theoretical-caveat mitigation — slow β change rate reduces instability risk from adaptive β breaking Adam's constant-β convergence proof. Pearl A sentinel 0 at fold boundary fires first-observation replacement (cosine_sim=0 → low-stability envelope endpoints). Aux groups 3-7 (IQN/IqlHigh/IqlLow/Attn/Curiosity) have non-contiguous grad allocations; zero-width sentinel offsets `[tp, tp, tp, tp, tp]` in `group_param_offsets_buf` prevent the kernel inner loop from executing for those groups, yielding cosine_sim=0 → β1/β2 start at low-stability floors. `apply_pearls_kernel.cu` signature gained `float alpha_meta` parameter (migration of all 18 call sites atomic per `feedback_no_partial_refactor.md`: 1 in `gpu_experience_collector.rs`, 17 in `gpu_dqn_trainer.rs`). Buffer growth: `producer_step_scratch_buf` 131→171 slots (SP5_SCRATCH_TOTAL); 5 new scratch constants `SCRATCH_PEARL_4_{COSINE=131, L2=139, BETA1=147, BETA2=155, EPS=163}`. StateResetRegistry gains 4 new FoldReset entries: `sp5_adam_beta1` (ISV[226..234)), `sp5_adam_beta2` (ISV[234..242)), `sp5_adam_eps` (ISV[242..250)), `sp5_grad_prev_buf` (grad_prev_buf_per_group — zero at fold boundary so cosine_sim=0 fires Pearl A bootstrap). Wire-up in training_loop.rs: `launch_sp5_pearl_4_adam_hparams()` called immediately after `launch_sp5_pearl_2_budget()` with `tracing::warn!` on error. Two GPU-only `#[ignore = "requires GPU"]` unit tests: (7) high-stability (cosine_sim=1.0 → β1=0.95, β2=0.9995, ε within envelope); (8) low-stability (cosine_sim=0.0 → β1=0.85, β2=0.99, ε within envelope). No CPU compute, no HtoD/HtoH, no atomicAdd, no stubs, no consumer migration. Touched: `cuda_pipeline/grad_cosine_sim_kernel.cu` (new), `cuda_pipeline/pearl_4_adam_hparams_kernel.cu` (new), `cuda_pipeline/apply_pearls_kernel.cu` (+alpha_meta param), `cuda_pipeline/sp4_wiener_ema.rs` (+alpha_meta arg), `cuda_pipeline/gpu_experience_collector.rs` (+1 call site ALPHA_META arg), `build.rs` (+2 cubin entries), `cuda_pipeline/gpu_dqn_trainer.rs` (SP5_SCRATCH_TOTAL 131→171 + 5 new constants + 2 static cubins + 4 struct fields + constructor allocs + struct initializer + 17 call-site ALPHA_META args + launch method), `trainers/dqn/state_reset_registry.rs` (+4 FoldReset entries), `trainers/dqn/trainer/training_loop.rs` (+5 LOC after Pearl 2 launch), `tests/sp5_producer_unit_tests.rs` (+2 GPU-only tests + cubin constant + loader helper). Hard rules: feedback_no_cpu_compute_strict, feedback_no_atomicadd, feedback_no_htod_htoh_only_mapped_pinned, feedback_no_stubs, feedback_no_partial_refactor (apply_pearls 18 call sites all migrated).