From 15818dce0123d8fecae977116db36fef8e4e2fa1 Mon Sep 17 00:00:00 2001 From: jgrusewski Date: Thu, 23 Apr 2026 22:25:07 +0200 Subject: [PATCH] fix(dqn): break cold-start q_std latch in update_eval_v_range MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause of Q-value saturation at +/-50 seen in train-6nbx5 after ISV v-range unification (9deda5f65, 11df03785): cold-start path in `update_eval_v_range` latched `q_std_ema = q_std.max(0.01)` on the first epoch. But that first `q_std` is dominated by the +/-50 bootstrap atom-spread (scaffolding set in construct/reset), NOT by real Q-distribution spread. Result: `3*std_ema` exceeds `min_half_floor=10` and approaches `abs_half=50` immediately, atoms stay wide next epoch, next `q_std` confirms that width, EMA never escapes. Q saturated at +/-abs_half every run. Fix 1 (gpu_dqn_trainer.rs:3106-3120): seed `eval_q_std_ema = min_half_floor / 3.0` at cold start so initial `half = 3 * std_ema = min_half_floor` exactly. Adaptive-rate EMA (alpha clamped to [0.01, 0.3]) then relaxes upward only if genuine Q-spread warrants it. Breaks the self-confirming initialization. Fix 2 (training_loop.rs:479-494): remove leftover pre-clamp of reward quantiles to `config.v_{min,max}`. That was from the earlier quantile-clamp fix (d38a8cf99). Phase 2c (9deda5f65) moved the per-branch clamp inside `warm_start_atom_positions`, which reads each branch's [centre-half, centre+half] from the ISV pinned bus. An outer static clamp to the wider config bound is redundant double-clamping and hides which layer owns the support. Pass raw quantiles through to warm_start. Validated locally: SQLX_OFFLINE cargo check -p ml passes (only pre-existing warnings). Next: push + L40S validation run. Diagnostic instrumentation from 423ac460b remains in place to confirm (center, half) trajectory on the next run — will be removed once validated. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../ml/src/cuda_pipeline/gpu_dqn_trainer.rs | 14 +++++++++- .../src/trainers/dqn/trainer/training_loop.rs | 27 ++++++++----------- 2 files changed, 24 insertions(+), 17 deletions(-) diff --git a/crates/ml/src/cuda_pipeline/gpu_dqn_trainer.rs b/crates/ml/src/cuda_pipeline/gpu_dqn_trainer.rs index cdc9f0ca3..58165990c 100644 --- a/crates/ml/src/cuda_pipeline/gpu_dqn_trainer.rs +++ b/crates/ml/src/cuda_pipeline/gpu_dqn_trainer.rs @@ -3103,9 +3103,21 @@ impl GpuDqnTrainer { let initialized_before = self.eval_ema_initialized[branch_idx]; // Adaptive-rate EMA with baseline proportional to Q-std. + // + // Cold-start seeding: the first observed `q_std` at epoch end is + // dominated by the ±bootstrap atom spread (the scaffolding we set + // in `construct` / `reset_eval_v_range_state`), NOT by the true + // Q-distribution spread. Latching it makes `3*std_ema` exceed + // `min_half_floor` and approach `abs_half`, so atoms stay wide + // next epoch, the next `q_std` confirms that width, and the EMA + // never escapes — Q saturates at ±abs_half. + // + // Seed conservatively so the initial `half` equals exactly + // `min_half_floor`. The adaptive-rate EMA then relaxes upward + // only if genuine Q-spread warrants it. if !self.eval_ema_initialized[branch_idx] { self.eval_q_mean_ema[branch_idx] = 0.0; - self.eval_q_std_ema[branch_idx] = q_std.max(0.01); + self.eval_q_std_ema[branch_idx] = min_half_floor / 3.0; self.eval_ema_initialized[branch_idx] = true; } else { let baseline = self.eval_q_std_ema[branch_idx].max(0.001); diff --git a/crates/ml/src/trainers/dqn/trainer/training_loop.rs b/crates/ml/src/trainers/dqn/trainer/training_loop.rs index aaac8ceeb..7327f415c 100644 --- a/crates/ml/src/trainers/dqn/trainer/training_loop.rs +++ b/crates/ml/src/trainers/dqn/trainer/training_loop.rs @@ -477,23 +477,18 @@ impl DQNTrainer { let num_atoms = self.hyperparams.num_atoms; match collector.compute_reward_quantiles(count, num_atoms) { Ok(quantiles) => { - // Raw reward quantiles are unbounded — a single extreme - // PnL sample from the environment becomes `atom_positions[50]`, - // and the C51 expected-value read `Q = Σ prob × atom_pos` - // inherits that magnitude. Observed on L40S: epoch 1 Q in - // [0, 6], epoch 2 Q at ±333k — the tail of the sorted - // reward distribution from the first experience buffer. - // Clamp to the configured `[v_min, v_max]` so warm-start - // respects the C51 support bounds that every downstream - // path (loss projection, eval_v_range, IQL support) assumes. - let v_min = self.hyperparams.v_min as f32; - let v_max = self.hyperparams.v_max as f32; - let clamped_quantiles: Vec = quantiles - .iter() - .map(|&q| q.clamp(v_min, v_max)) - .collect(); + // Per-branch clamp now lives inside + // `warm_start_atom_positions` (ISV v-range spec Phase 2c, + // commit 9deda5f65). That path reads each branch's + // `[centre - half, centre + half]` from the ISV pinned + // bus and clamps the quantile array per branch. An + // outer static clamp to `config.v_{min,max}` here would + // be redundant double-clamping to a wider bound — it + // hides which layer owns the support and never + // contributes once ISV is populated. Pass the raw + // quantiles straight through. if let Some(ref mut fused) = self.fused_ctx { - if let Err(e) = fused.warm_start_atom_positions(&clamped_quantiles) { + if let Err(e) = fused.warm_start_atom_positions(&quantiles) { warn!("C51 atom warm-start failed (non-fatal): {e}"); } }