From c07197f54cedbb2295bf37cb25dbc6c3b1fc8357 Mon Sep 17 00:00:00 2001 From: jgrusewski Date: Thu, 28 May 2026 15:44:15 +0200 Subject: [PATCH] fix(rl): remove double-divide on l_q reading (cosmetic, matches l_pi) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit dqn_distributional_q.cu:285 atomicAdds ce/B into ss_q_loss (kernel-side B divide, matching ppo_clipped_surrogate.cu:269-271's b_inv pattern). Host previously divided by B AGAIN at integrated.rs:3944 and :5196 — true mean CE (≈ 3.0 for 21-atom softmax) appeared as 3.0/1024 ≈ 0.003. The "baseline l_q=1.45 at step 500" was actually a missing-zero-out artifact: ss_q_loss accumulated 500 × 0.003 = 1.46 across steps before bf8887400 fixed the zero-out, exposing the underlying double-divide. Q learning has been working correctly the whole time — only the diagnostic display was wrong. Co-Authored-By: Claude Opus 4.7 --- crates/ml-alpha/src/trainer/integrated.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/crates/ml-alpha/src/trainer/integrated.rs b/crates/ml-alpha/src/trainer/integrated.rs index a83fd4da2..b70491f41 100644 --- a/crates/ml-alpha/src/trainer/integrated.rs +++ b/crates/ml-alpha/src/trainer/integrated.rs @@ -3941,7 +3941,11 @@ impl IntegratedTrainer { self.last_pi_loss = l_pi_host; let l_q_host = unsafe { std::ptr::read_volatile(self.ss_q_loss_mapped.host_ptr) }; - self.last_q_loss = l_q_host / (b_size as f32); + // Kernel already divides by B in atomicAdd (dqn_distributional_q.cu:285). + // The previous host divide was a double-divide bug — true mean CE + // appeared as mean/B ≈ 0.003 instead of ≈ 3.0. Matches l_pi convention + // (ppo_clipped_surrogate.cu:269-271 also kernel-side divides). + self.last_q_loss = l_q_host; let l_v_sum_host = unsafe { std::ptr::read_volatile(self.ss_v_loss_sum_mapped.host_ptr) }; let l_v_host = l_v_sum_host / (b_size as f32); @@ -5193,7 +5197,8 @@ impl IntegratedTrainer { // is zero = sentinel, and the per-branch LR controller's cold- // start gate handles that. let l_q_host = unsafe { std::ptr::read_volatile(self.ss_q_loss_mapped.host_ptr) }; - let l_q = l_q_host / (b_size as f32).max(1.0); + // Kernel divides by B (see step_synthetic_body comment). No host divide. + let l_q = l_q_host; self.dqn_head .backward_gemm(