diff --git a/crates/ml/src/trainers/dqn/trainer/training_loop.rs b/crates/ml/src/trainers/dqn/trainer/training_loop.rs index 198558ad4..a2c3cd174 100644 --- a/crates/ml/src/trainers/dqn/trainer/training_loop.rs +++ b/crates/ml/src/trainers/dqn/trainer/training_loop.rs @@ -3423,6 +3423,59 @@ impl DQNTrainer { "SP9 kelly_warmup_floor producer chain launch failed"); } + // C.2 Reward-component attribution EMA (Plan 3 + // Task 1, spec §4.C.2). GPU-side: launch + // `reward_component_ema` kernel (single-block, + // 6 threads). Reads `reward_components_per_sample` + // [N*L, 6] written by `collect_experiences_gpu`, + // reduces mean(|r_c|) into ISV[63..69) (popart at + // 63, cf 64, trail 65, micro 66, opp_cost 67, + // bonus 68) AND per-component variance via + // Welford into ISV[362..367) (cf var 362 .. + // bonus var 366; popart-var lives at 361 and is + // produced by `launch_sp11_popart_component_ema` + // below). Resets the per-sample component buffer. + // + // SP11 launch-order fix-up (2026-05-04): MUST run + // BEFORE the SP11 canary chain below — the mag- + // ratio canary at `launch_sp11_mag_ratio_compute` + // reads ISV[REWARD_POPART_EMA_INDEX+1..+6) = + // [64..68] (mag) and ISV[REWARD_COMPONENT_VAR_ + // EMA_BASE+1..+6) = [362..367) (var) for the + // non-popart axes (1..5). Previously this kernel + // launched at the C.2 site below the canary, + // leaving those slots at sentinel-0 on ep1 (ep0 + // had no segment_complete fires) → z[c]=0 for + // c=1..5 → ratio[0]=1.0 → popart dominates → + // controller saturates → sharpe collapses. Smoke + // `smoke-test-4rbv9` on b3b4d0278 confirmed the + // structurally-no-op behaviour. The kernel itself + // is unchanged; only the launch site moved. + // Other launches at the original C.2 site + // (trade_attempt_rate, plan_threshold, + // seed_step_counter, cql_alpha_seed) stay there — + // different consumers, no pre-canary constraint. + // Per `feedback_no_partial_refactor.md`, this is + // a single atomic move. ISV pointer must be + // non-zero; noop otherwise (no collector or ISV + // not set). + { + let n_main = self.gpu_experience_collector + .as_ref() + .map(|c| c.alloc_episodes() * c.alloc_timesteps()) + .unwrap_or(0); + if let Some(ref mut c) = self.gpu_experience_collector { + if let Err(e) = + c.launch_reward_component_ema_inplace(n_main) + { + tracing::warn!( + "C.2 reward_component_ema launch failed \ + (SP11 canary input): {e}" + ); + } + } + } + // SP11 Fix 39 B1b fix-up (2026-05-04): popart- // component-specific magnitude EMA producer. // Reads the per-bar `popart_component_per_sample` @@ -3687,26 +3740,33 @@ impl DQNTrainer { }, None => [0.0_f32; 4], }; - // C.2 Reward-component attribution EMA (Plan 3 Task 1, spec §4.C.2). - // GPU-side: launch reward_component_ema kernel (single-block, 6 threads). - // Reads reward_components_per_sample [N*L, 6] written by collect_experiences_gpu, - // reduces mean |r_c|, EMA into ISV[63..69). Resets the component buffer. - // Must run BEFORE trail_fire_and_hold_per_mag so the buffer is still populated. - // ISV pointer must be non-zero; noop otherwise (no collector or ISV not set). - // - // B.2 Plan 3 Task 3 (spec §4.B.2): same launch site for the + // B.2 Plan 3 Task 3 (spec §4.B.2): launch site for the // trade_attempt_rate_ema kernel — reads flat_to_pos_per_sample and // adaptive-EMA-updates ISV[TRADE_ATTEMPT_RATE_EMA_INDEX]. α_base // matches the reward-EMA convention (0.05). + // + // SP11 launch-order fix-up (2026-05-04): the C.2 + // `launch_reward_component_ema_inplace` kernel that previously + // ran here was MOVED to before the SP11 canary chain (see the + // SP11 producer block above, just before + // `launch_sp11_popart_component_ema`). Reason: the SP11 mag- + // ratio canary reads ISV[64..68] (cf/trail/micro/opp_cost/ + // bonus mag EMAs) and ISV[362..366] (paired variances), both + // of which are POPULATED by `launch_reward_component_ema_inplace`. + // With the kernel still here, those slots held stale (or + // sentinel-0) values when the canary fired earlier in the same + // step → z-score collapsed to the linear formula and B1b + // smoke `smoke-test-4rbv9` saw bit-identical w_pop=2.0 to the + // pre-z-score linear-formula smoke. The other launches in this + // block (trade_attempt_rate, plan_threshold, seed_step_counter, + // cql_alpha_seed) STAY here — their consumers fire elsewhere + // in the loop and have no pre-canary timing constraint. { let n_main = self.gpu_experience_collector.as_ref().map(|c| { c.alloc_episodes() * c.alloc_timesteps() }).unwrap_or(0); let ema_alpha = 0.05_f32; if let Some(ref mut c) = self.gpu_experience_collector { - if let Err(e) = c.launch_reward_component_ema_inplace(n_main) { - tracing::warn!("C.2 reward_component_ema launch failed: {e}"); - } if let Err(e) = c.launch_trade_attempt_rate_ema_inplace(n_main, ema_alpha) { tracing::warn!("B.2 trade_attempt_rate_ema launch failed: {e}"); } diff --git a/docs/dqn-wire-up-audit.md b/docs/dqn-wire-up-audit.md index 01cddabe7..e8f54aa11 100644 --- a/docs/dqn-wire-up-audit.md +++ b/docs/dqn-wire-up-audit.md @@ -5351,3 +5351,84 @@ After this fix-up, all three known SP11 reward-system bugs surfaced by deep audit are resolved (slot 63 overload, stale rc[] init, cf_flip ordering, cf-component feedback loop). L40S smoke validates the empirical sharpe recovery. + +## SP11 B1b launch-order fix-up — reward_component_ema before mag-ratio canary (2026-05-04) + +Smoke `smoke-test-4rbv9` on the z-score implementation (commit +`b3b4d0278`) showed bit-identical `w_pop=2.000` at ep1 to the +pre-z-score B1b smoke. Investigation proved the z-score formula +was structurally a no-op when ISV variance slots `[361..367)` were +sentinel-0: + +``` +z[c] = mag[c] / fmaxf(sqrtf(0), EPS_DIV) = mag[c] × 1e6 +sum_z = Σ (mag[c] × 1e6) = 1e6 × Σ mag[c] +ratio = (mag[c] × 1e6) / (1e6 × Σ mag) = mag[c] / Σ mag ← linear +``` + +Root cause: `launch_reward_component_ema_inplace` (the kernel +populating ISV[64..68] mag and ISV[362..366] var for axes +1..5 = cf/trail/micro/opp_cost/bonus) ran at the C.2 site +(`crates/ml/src/trainers/dqn/trainer/training_loop.rs:3707`) +AFTER `launch_sp11_mag_ratio_compute` at line 3465. So the mag- +ratio canary read sentinel-0 at ep1 (ep0 had no segment_complete +fires) → z[c]=0 for c=1..5 → ratio[0]=1.0 → controller saturated. +This was structurally identical to the slot-63 PopArt overload +bug that motivated the B1b `launch_sp11_popart_component_ema` +fix-up — popart was patched at slot 360, but cf/trail/micro/ +opp_cost/bonus were left to read stale slots 64..68 + 362..366. + +Fix: moved `launch_reward_component_ema_inplace` from the C.2 site +(line ~3707) to immediately BEFORE `launch_sp11_popart_component_ema` +in the SP11 producer chain (line ~3441). The kernel itself is +unchanged — only the launch site moved. The other launches at the +original C.2 site (`launch_trade_attempt_rate_ema_inplace`, +`launch_plan_threshold_update_inplace`, +`launch_seed_step_counter_update_inplace`, +`launch_cql_alpha_seed_update_inplace`) STAY at C.2 — their +consumers fire elsewhere in the loop with no pre-canary timing +constraint. + +Pre-conditions verified before the move: + +1. `collect_experiences_gpu` (line ~1822) populates + `reward_components_per_sample` BEFORE the SP11 producer chain + at line 3441 — so the input buffer the kernel reads is fresh. +2. Between `launch_sp11_mag_ratio_compute` (line 3465) and the + old C.2 site (line 3707), no consumer reads ISV[64..68] or + ISV[362..366] for production decisions: + - `launch_sp11_saboteur_engagement_compute` reads + `PNL_REWARD_MAGNITUDE_EMA_INDEX=359`, not the per-component + mag/var slots. + - `launch_sp11_reward_subsystem_controller` reads canary + outputs at `[350..360)`, not the inputs. + - The first reads of slots 64..68 on the host happen at + `training_loop.rs:4540..4545` (HEALTH_DIAG emit) AFTER + the move target. + So no consumer between the two sites depends on stale values. + +Per `feedback_no_partial_refactor.md`, the move is a single +atomic commit. Per `feedback_no_quickfixes.md`, this IS the +proper fix — the kernel was always supposed to run before the +canary; the C.2 site just predated the SP11 chain. Per +`feedback_no_legacy_aliases.md`, no shim or compatibility wrapper +left at the old site. + +Comments updated at both sites with the launch-order rationale +and a back-reference to `smoke-test-4rbv9` so the next agent +auditing this code path doesn't have to re-derive it. + +Verified post-fix: + +- `SQLX_OFFLINE=true cargo check -p ml --lib` — clean +- `SQLX_OFFLINE=true cargo test -p ml --lib trainers::dqn::monitors::reward_component_monitor` — 3/3 passing +- The 9 pre-existing test failures in `trainers::dqn::trainer::tests` + / `state_kl_monitor` / `gradient_budget` reproduce on HEAD + (commit `b3b4d0278`) without my change — they are due to + test fixtures missing OFI features (`Some(OFI features missing + — model requires order flow data)`), not regressions introduced + by this commit. + +Follow-up: re-run B1b smoke on the patched commit; expectation is +`w_pop` no longer pinning to 2.0 at ep1 because z[c] for c=1..5 +now sees populated mag and var at the moment the canary fires.