gitadmin/foxhunt
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(sp5): Layer B fix-up — close 3 review findings before Layer C

Browse Source 
Comprehensive review of Layer B (commit 99367b9c6) caught one critical
and two important findings. All three fix in one atomic commit.

Critical: IQL Adam groups 4+5 (IqlHigh, IqlLow) missed the Pearl 4
migration. gpu_iql_trainer.rs::train_value_step still read
self.config.beta1=0.9, self.config.beta2=0.999 at runtime. ISV[230]
(adam_beta1(4)) and ISV[231] (adam_beta1(5)) were populated by the
Layer A producer but unconsumed — partial refactor of the 8-group
Pearl 4 contract. Fix mirrors the Curiosity pattern: 3 new
iql_beta1/beta2/epsilon parameters threaded through the IQL Adam call,
2 ISV reads at each of the 2 train_value_step call sites in
fused_training.rs.

Important: consumer clamp envelopes were wider than the SP4 producer
range (0.5..0.9999 vs producer's 0.85..0.95 for β1, etc.). At
cold-start with ISV=0, this produced β1=0.5 (more aggressive momentum
than the SP4-anchored 0.85 floor). Tightened all Pearl 4 consumer
clamps to match the producer envelope: β1∈[0.85,0.95], β2∈[0.99,0.9995],
ε∈[1e-10,1e-6].

Important: gpu_curiosity_trainer.rs:66-68 left dead ADAM_BETA1/BETA2/
EPS constants after the Pearl 4 migration. Removed.

8/8 Pearl 4 groups now ISV-driven (DqnTrunk, Value, Branches, IQN,
IqlHigh, IqlLow, Attn, Curiosity). cargo check + cargo build + lib
tests all clean.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
jgrusewski
2026-05-02 01:27:40 +02:00
parent 99367b9c60
commit 3ad5e011b9
5 changed files with 62 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
crates/ml/src/cuda_pipeline/gpu_curiosity_trainer.rs
View File

@@ -63,9 +63,6 @@ const CUR_B2_LEN: usize = CUR_OUTPUT; // [42]
/// Adam optimizer hyperparameters.
const ADAM_LR: f32 = 0.001;
const ADAM_BETA1: f32 = 0.9;
const ADAM_BETA2: f32 = 0.999;
const ADAM_EPS: f32 = 1e-8;
// ---------------------------------------------------------------------------
// Minimal cuBLAS context for the curiosity trainer

12
crates/ml/src/cuda_pipeline/gpu_iql_trainer.rs
View File

@@ -588,6 +588,12 @@ impl GpuIqlTrainer {
/* SP4 Layer B (Mech 9): ISV-driven AdamW weight_decay rate.
* Caller reads `ISV[WD_RATE[group]]` with `.max(0.0)` non-negativity. */
weight_decay_value: f32,
/* SP5 Pearl 4: per-group Adam β1/β2/ε read from ISV[ADAM_BETA1_BASE+g],
* ISV[ADAM_BETA2_BASE+g], ISV[ADAM_EPS_BASE+g] by FusedTrainingCtx
* and threaded through. g=4 for IqlHigh, g=5 for IqlLow. */
iql_beta1: f32,
iql_beta2: f32,
iql_epsilon: f32,
) -> Result<(), MLError> {
let b = self.config.batch_size;
let h = self.config.value_hidden_dim;
@@ -1036,9 +1042,9 @@ impl GpuIqlTrainer {
unsafe { *self.t_pinned = self.adam_step; }
let adam_blocks = (self.total_params + 255) / 256;
let lr = self.config.lr;
let beta1 = self.config.beta1;
let beta2 = self.config.beta2;
let eps = self.config.epsilon;
let beta1 = iql_beta1;
let beta2 = iql_beta2;
let eps = iql_epsilon;
// SP4 Layer B (Mech 9): AdamW weight_decay sourced from
// ISV[WD_RATE[IqlHigh|IqlLow]] by the caller (`FusedTrainingCtx`)
// and threaded through `train_value_step`. `.max(0.0)` upstream

48
crates/ml/src/trainers/dqn/fused_training.rs
View File

@@ -1633,9 +1633,9 @@ impl FusedTrainingCtx {
self.trainer.read_group_adam_bounds(crate::cuda_pipeline::ParamGroup::Attn);
// SP5 Layer B (Pearl 4): TLOB shares ParamGroup::Attn=6.
let tlob_g = crate::cuda_pipeline::ParamGroup::Attn.idx();
let tlob_beta1 = self.trainer.read_isv_signal_at(ADAM_BETA1_BASE + tlob_g).max(0.5_f32).min(0.9999_f32);
let tlob_beta2 = self.trainer.read_isv_signal_at(ADAM_BETA2_BASE + tlob_g).max(0.9_f32).min(0.99999_f32);
let tlob_epsilon = self.trainer.read_isv_signal_at(ADAM_EPS_BASE + tlob_g).max(1e-12_f32);
let tlob_beta1 = self.trainer.read_isv_signal_at(ADAM_BETA1_BASE + tlob_g).clamp(0.85_f32, 0.95_f32);
let tlob_beta2 = self.trainer.read_isv_signal_at(ADAM_BETA2_BASE + tlob_g).clamp(0.99_f32, 0.9995_f32);
let tlob_epsilon = self.trainer.read_isv_signal_at(ADAM_EPS_BASE + tlob_g).clamp(1e-10_f32, 1e-6_f32);
self.gpu_tlob
.adam_step(
self.trainer.config().lr,
@@ -1942,11 +1942,25 @@ impl FusedTrainingCtx {
self.trainer.read_group_adam_bounds(crate::cuda_pipeline::ParamGroup::IqlHigh);
let (iql_low_clamp, iql_low_wd) =
self.trainer.read_group_adam_bounds(crate::cuda_pipeline::ParamGroup::IqlLow);
// SP5 Pearl 4: per-group Adam β1/β2/ε for IqlHigh (group 4) and IqlLow (group 5).
// ISV[ADAM_BETA1_BASE+4=230], ISV[ADAM_BETA2_BASE+4=238], ISV[ADAM_EPS_BASE+4=246].
// ISV[ADAM_BETA1_BASE+5=231], ISV[ADAM_BETA2_BASE+5=239], ISV[ADAM_EPS_BASE+5=247].
// Clamps match SP4 producer envelope: β1∈[0.85,0.95], β2∈[0.99,0.9995], ε∈[1e-10,1e-6].
let iql_high_g = crate::cuda_pipeline::ParamGroup::IqlHigh.idx();
let iql_high_beta1 = self.trainer.read_isv_signal_at(ADAM_BETA1_BASE + iql_high_g).clamp(0.85_f32, 0.95_f32);
let iql_high_beta2 = self.trainer.read_isv_signal_at(ADAM_BETA2_BASE + iql_high_g).clamp(0.99_f32, 0.9995_f32);
let iql_high_epsilon = self.trainer.read_isv_signal_at(ADAM_EPS_BASE + iql_high_g).clamp(1e-10_f32, 1e-6_f32);
let iql_low_g = crate::cuda_pipeline::ParamGroup::IqlLow.idx();
let iql_low_beta1 = self.trainer.read_isv_signal_at(ADAM_BETA1_BASE + iql_low_g).clamp(0.85_f32, 0.95_f32);
let iql_low_beta2 = self.trainer.read_isv_signal_at(ADAM_BETA2_BASE + iql_low_g).clamp(0.99_f32, 0.9995_f32);
let iql_low_epsilon = self.trainer.read_isv_signal_at(ADAM_EPS_BASE + iql_low_g).clamp(1e-10_f32, 1e-6_f32);
let states_f32 = self.trainer.states_buf();
self.gpu_iql.train_value_step(states_f32, iql_high_clamp, iql_high_wd)
self.gpu_iql.train_value_step(states_f32, iql_high_clamp, iql_high_wd,
iql_high_beta1, iql_high_beta2, iql_high_epsilon)
.map_err(|e| anyhow::anyhow!("IQL high train: {e}"))?;
let states_f32 = self.trainer.states_buf();
self.gpu_iql_low.train_value_step(states_f32, iql_low_clamp, iql_low_wd)
self.gpu_iql_low.train_value_step(states_f32, iql_low_clamp, iql_low_wd,
iql_low_beta1, iql_low_beta2, iql_low_epsilon)
.map_err(|e| anyhow::anyhow!("IQL low train: {e}"))?;
// 4. Compute advantage weights (from high-tau V(s)).
@@ -2059,9 +2073,9 @@ impl FusedTrainingCtx {
self.trainer.read_group_adam_bounds(crate::cuda_pipeline::ParamGroup::Iqn);
// SP5 Layer B (Pearl 4): per-group Adam β1/β2/ε for Iqn=3.
let iqn_g = crate::cuda_pipeline::ParamGroup::Iqn.idx();
let iqn_beta1 = self.trainer.read_isv_signal_at(ADAM_BETA1_BASE + iqn_g).max(0.5_f32).min(0.9999_f32);
let iqn_beta2 = self.trainer.read_isv_signal_at(ADAM_BETA2_BASE + iqn_g).max(0.9_f32).min(0.99999_f32);
let iqn_epsilon = self.trainer.read_isv_signal_at(ADAM_EPS_BASE + iqn_g).max(1e-12_f32);
let iqn_beta1 = self.trainer.read_isv_signal_at(ADAM_BETA1_BASE + iqn_g).clamp(0.85_f32, 0.95_f32);
let iqn_beta2 = self.trainer.read_isv_signal_at(ADAM_BETA2_BASE + iqn_g).clamp(0.99_f32, 0.9995_f32);
let iqn_epsilon = self.trainer.read_isv_signal_at(ADAM_EPS_BASE + iqn_g).clamp(1e-10_f32, 1e-6_f32);
match iqn.execute_training_pipeline(
online_h_s2, next_states_buf, &self.target_dueling,
@@ -2149,9 +2163,9 @@ impl FusedTrainingCtx {
self.trainer.read_group_adam_bounds(crate::cuda_pipeline::ParamGroup::Attn);
// SP5 Layer B (Pearl 4): per-group Adam β1/β2/ε for Attn=6.
let attn_g = crate::cuda_pipeline::ParamGroup::Attn.idx();
let attn_beta1 = self.trainer.read_isv_signal_at(ADAM_BETA1_BASE + attn_g).max(0.5_f32).min(0.9999_f32);
let attn_beta2 = self.trainer.read_isv_signal_at(ADAM_BETA2_BASE + attn_g).max(0.9_f32).min(0.99999_f32);
let attn_epsilon = self.trainer.read_isv_signal_at(ADAM_EPS_BASE + attn_g).max(1e-12_f32);
let attn_beta1 = self.trainer.read_isv_signal_at(ADAM_BETA1_BASE + attn_g).clamp(0.85_f32, 0.95_f32);
let attn_beta2 = self.trainer.read_isv_signal_at(ADAM_BETA2_BASE + attn_g).clamp(0.99_f32, 0.9995_f32);
let attn_epsilon = self.trainer.read_isv_signal_at(ADAM_EPS_BASE + attn_g).clamp(1e-10_f32, 1e-6_f32);
attn.adam_step(lr, mgn, Some(&attn_stream_ref), ws_override,
attn_weight_clamp_max_abs, attn_weight_decay_value,
attn_beta1, attn_beta2, attn_epsilon)
@@ -2193,9 +2207,9 @@ impl FusedTrainingCtx {
self.trainer.read_group_adam_bounds(crate::cuda_pipeline::ParamGroup::Attn);
// SP5 Layer B (Pearl 4): per-group Adam β1/β2/ε for Attn=6.
let attn_g = crate::cuda_pipeline::ParamGroup::Attn.idx();
let attn_beta1 = self.trainer.read_isv_signal_at(ADAM_BETA1_BASE + attn_g).max(0.5_f32).min(0.9999_f32);
let attn_beta2 = self.trainer.read_isv_signal_at(ADAM_BETA2_BASE + attn_g).max(0.9_f32).min(0.99999_f32);
let attn_epsilon = self.trainer.read_isv_signal_at(ADAM_EPS_BASE + attn_g).max(1e-12_f32);
let attn_beta1 = self.trainer.read_isv_signal_at(ADAM_BETA1_BASE + attn_g).clamp(0.85_f32, 0.95_f32);
let attn_beta2 = self.trainer.read_isv_signal_at(ADAM_BETA2_BASE + attn_g).clamp(0.99_f32, 0.9995_f32);
let attn_epsilon = self.trainer.read_isv_signal_at(ADAM_EPS_BASE + attn_g).clamp(1e-10_f32, 1e-6_f32);
attn.adam_step(lr, mgn, None, None,
attn_weight_clamp_max_abs, attn_weight_decay_value,
attn_beta1, attn_beta2, attn_epsilon)
@@ -2213,9 +2227,9 @@ impl FusedTrainingCtx {
self.trainer.read_group_adam_bounds(crate::cuda_pipeline::ParamGroup::Iqn);
// SP5 Layer B (Pearl 4): per-group Adam β1/β2/ε for Iqn=3.
let iqn_g = crate::cuda_pipeline::ParamGroup::Iqn.idx();
let iqn_beta1 = self.trainer.read_isv_signal_at(ADAM_BETA1_BASE + iqn_g).max(0.5_f32).min(0.9999_f32);
let iqn_beta2 = self.trainer.read_isv_signal_at(ADAM_BETA2_BASE + iqn_g).max(0.9_f32).min(0.99999_f32);
let iqn_epsilon = self.trainer.read_isv_signal_at(ADAM_EPS_BASE + iqn_g).max(1e-12_f32);
let iqn_beta1 = self.trainer.read_isv_signal_at(ADAM_BETA1_BASE + iqn_g).clamp(0.85_f32, 0.95_f32);
let iqn_beta2 = self.trainer.read_isv_signal_at(ADAM_BETA2_BASE + iqn_g).clamp(0.99_f32, 0.9995_f32);
let iqn_epsilon = self.trainer.read_isv_signal_at(ADAM_EPS_BASE + iqn_g).clamp(1e-10_f32, 1e-6_f32);
match iqn.execute_training_pipeline(
online_h_s2, next_states_buf, &self.target_dueling,

6
crates/ml/src/trainers/dqn/trainer/training_loop.rs
View File

@@ -1816,9 +1816,9 @@ impl DQNTrainer {
let cur_g = crate::cuda_pipeline::ParamGroup::Curiosity.idx();
let (cur_beta1, cur_beta2, cur_epsilon) = if let Some(ref fused) = self.fused_ctx {
(
fused.read_isv_signal_at(SP5_ADAM_BETA1_BASE + cur_g).max(0.5_f32).min(0.9999_f32),
fused.read_isv_signal_at(SP5_ADAM_BETA2_BASE + cur_g).max(0.9_f32).min(0.99999_f32),
fused.read_isv_signal_at(SP5_ADAM_EPS_BASE + cur_g).max(1e-12_f32),
fused.read_isv_signal_at(SP5_ADAM_BETA1_BASE + cur_g).clamp(0.85_f32, 0.95_f32),
fused.read_isv_signal_at(SP5_ADAM_BETA2_BASE + cur_g).clamp(0.99_f32, 0.9995_f32),
fused.read_isv_signal_at(SP5_ADAM_EPS_BASE + cur_g).clamp(1e-10_f32, 1e-6_f32),
)
} else {
(0.9_f32, 0.999_f32, 1e-8_f32)

19
docs/dqn-wire-up-audit.md
View File

@@ -3229,3 +3229,22 @@ All 11 consumer sites migrated in one atomic commit per `feedback_no_partial_ref
- `cargo test -p ml --lib` → 930 pass, 14 pre-existing failures unchanged
Refs: `feedback_no_partial_refactor`, `feedback_isv_for_adaptive_bounds`, `feedback_adaptive_not_tuned`
### SP5 Layer B fix-up (2026-05-01)
Three review findings on Layer B (commit `99367b9c6`) resolved in one atomic commit before Layer C validation.
**Critical — IQL groups 4+5 complete Pearl 4 migration:**
`gpu_iql_trainer.rs::train_value_step` still read `self.config.beta1=0.9`, `self.config.beta2=0.999`, `self.config.epsilon` at runtime (lines 1039-1041). ISV[230] (adam_beta1(4)), ISV[231] (adam_beta1(5)), ISV[238], ISV[239], ISV[246], ISV[247] were populated by the Layer A producer but unconsumed — partial Pearl 4 contract with 6 of 8 groups migrated. Fix: `train_value_step` gains 3 new parameters (`iql_beta1`, `iql_beta2`, `iql_epsilon`); `fused_training.rs` reads ISV at both call sites using `ADAM_BETA1_BASE + iql_high_g/iql_low_g` and threads values through. Pearl 4 is now complete for all 8 param groups (DqnTrunk, DqnValue, DqnBranches, Iqn, IqlHigh, IqlLow, Attn, Curiosity).
**Important — consumer clamp envelopes tightened to SP4 producer range:**
All Pearl 4 consumer clamps across `fused_training.rs` (IQN, ATTN, TLOB sites, new IQL sites) and `training_loop.rs` (Curiosity site) tightened from `max(0.5).min(0.9999)` / `max(0.9).min(0.99999)` / `max(1e-12)` to match the SP4 producer envelope: β1∈[0.85, 0.95], β2∈[0.99, 0.9995], ε∈[1e-10, 1e-6]. Cold-start floor is now 0.85 (not 0.5) for β1, eliminating the overly-aggressive momentum at ISV=0.
**Important — dead constants removed:**
`gpu_curiosity_trainer.rs` lines 66-68: `const ADAM_BETA1: f32 = 0.9`, `const ADAM_BETA2: f32 = 0.999`, `const ADAM_EPS: f32 = 1e-8` removed. These were made dead by the Layer B Pearl 4 migration; the runtime path already read from threaded parameters.
**Verification:**
- `cargo check -p ml --offline` → clean (12 warnings, all pre-existing)
- `cargo test -p ml --lib -- sp5 sp4 state_reset_registry` → 13 pass, 0 fail
Refs: `feedback_no_partial_refactor`, `feedback_isv_for_adaptive_bounds`