merge: adaptive Q-drift kill threshold + real GPU IQN-sync test

- fe2b77388 adaptive kill threshold from ISV |Q| reference (no tuned constant)
- f2335b3e5 real GPU runtime test for sync_target_from_online (replaces static test)
This commit is contained in:
jgrusewski
2026-04-28 20:38:04 +02:00
4 changed files with 352 additions and 16 deletions

View File

@@ -1515,6 +1515,41 @@ impl GpuIqnHead {
Ok(())
}
/// Test-only borrow of the online IQN parameter buffer.
///
/// Only compiled under `cfg(test)` so this does not widen the public
/// API. Used by the GPU runtime test for `sync_target_from_online`
/// (see `tests::iqn_sync_target_from_online_makes_target_equal_online`)
/// to issue DtoD reads through `MappedF32Buffer`'s `dev_ptr` and
/// verify the buffer contents post-sync.
#[cfg(test)]
pub(crate) fn online_params_slice(&self) -> &CudaSlice<f32> {
&self.online_params
}
/// Test-only borrow of the target IQN parameter buffer.
/// See `online_params_slice` for the rationale.
#[cfg(test)]
pub(crate) fn target_params_slice(&self) -> &CudaSlice<f32> {
&self.target_params
}
/// Test-only accessor for the parameter count (`config.total_params()`
/// cached at construction).
#[cfg(test)]
pub(crate) fn total_params_for_test(&self) -> usize {
self.total_params
}
/// Test-only borrow of the owning CUDA stream — the same stream every
/// async DtoD copy on this head is queued against. The test issues its
/// fills and read-backs on this stream, then synchronises before
/// asserting on the mapped-pinned host pointer.
#[cfg(test)]
pub(crate) fn stream_for_test(&self) -> &Arc<CudaStream> {
&self.stream
}
/// Update target IQN weights via Polyak EMA.
///
/// `target[i] = (1 - tau) * target[i] + tau * online[i]`
@@ -2207,3 +2242,185 @@ fn clone_cuda_slice(
})?;
Ok(dst)
}
// ---------------------------------------------------------------------------
// Tests
// ---------------------------------------------------------------------------
#[cfg(test)]
mod tests {
//! GPU runtime regression tests for `GpuIqnHead`.
//!
//! All tests carry `#[ignore = "gpu"]` because they require a live CUDA
//! device. They run on the L40S smoke validation pool — `cargo test
//! -p ml --lib` on a CPU-only host skips them by default. Trigger
//! manually with:
//! cargo test -p ml --lib gpu_iqn_head -- --ignored --nocapture
//!
//! ## CPU↔GPU communication discipline
//!
//! Per `feedback_no_htod_htoh_only_mapped_pinned.md`, the only
//! permitted CPU↔GPU path is `cuMemHostAlloc(DEVICEMAP|PORTABLE)`
//! mapped pinned memory. Tests are not exempt — these tests fill and
//! read IQN parameter buffers through `MappedF32Buffer` and use
//! `cuMemcpyDtoDAsync` between the mapped buffer's `dev_ptr` and the
//! head's `online_params` / `target_params` `CudaSlice`s. No
//! `memcpy_dtoh` / `memcpy_htod` anywhere on the test path.
use super::*;
use crate::cuda_pipeline::mapped_pinned::MappedF32Buffer;
use crate::cuda_pipeline::shared_cublas_handle::PerStreamCublasHandles;
use cudarc::driver::CudaContext;
/// Fill an entire `CudaSlice<f32>` to a single scalar value via a
/// mapped-pinned staging buffer and `cuMemcpyDtoDAsync`. No HtoD copy
/// is issued; the host write through `MappedF32Buffer::host_ptr`
/// reaches the GPU through the device-mapped alias, then the DtoD
/// copies the staged values into `dst`.
///
/// Caller must `stream.synchronize()` before treating `dst` as
/// settled (or before reusing the staging buffer for another value).
fn fill_slice_via_mapped(
stream: &Arc<CudaStream>,
staging: &MappedF32Buffer,
dst: &CudaSlice<f32>,
n: usize,
value: f32,
) {
assert_eq!(staging.len, n, "staging buffer size must match dst");
// Host-side fill of the pinned mapping; the device sees the same
// bytes through `staging.dev_ptr`.
let host_slice: &mut [f32] = unsafe {
std::slice::from_raw_parts_mut(staging.host_ptr, n)
};
host_slice.fill(value);
let n_bytes = n * std::mem::size_of::<f32>();
unsafe {
cudarc::driver::result::memcpy_dtod_async(
dst.raw_ptr(),
staging.dev_ptr,
n_bytes,
stream.cu_stream(),
)
.expect("DtoD fill via mapped pinned staging");
}
}
/// Read an entire `CudaSlice<f32>` into a fresh `Vec<f32>` via a
/// mapped-pinned destination buffer and `cuMemcpyDtoDAsync`. Returns
/// after `stream.synchronize()` so the host read is coherent.
fn read_slice_via_mapped(
stream: &Arc<CudaStream>,
src: &CudaSlice<f32>,
n: usize,
) -> Vec<f32> {
let mapped = unsafe { MappedF32Buffer::new(n) }
.expect("MappedF32Buffer alloc for read-back");
let n_bytes = n * std::mem::size_of::<f32>();
unsafe {
cudarc::driver::result::memcpy_dtod_async(
mapped.dev_ptr,
src.raw_ptr(),
n_bytes,
stream.cu_stream(),
)
.expect("DtoD read-back into mapped pinned destination");
}
stream.synchronize().expect("stream sync after read-back");
mapped.read_all()
}
/// GPU runtime contract test for `GpuIqnHead::sync_target_from_online`.
///
/// The fold-boundary path in `fused_training.rs::reset_for_fold`
/// depends on `sync_target_from_online` performing a real DtoD copy
/// from `online_params` into `target_params`. Without it, the IQN
/// target buffer carries end-of-prior-fold weights into fold N+1 and
/// Polyak EMA at τ=0.005/step is far too slow to close the resulting
/// Bellman-target gap before Q drifts geometrically (commit
/// `7c19b5903`, issue #84).
///
/// This test exercises the contract end-to-end on a real GPU:
/// 1. Fill `online_params` ← 0.42 (f32) and `target_params` ← 0.99
/// via mapped-pinned staging + DtoD. `0.42` and `0.99` are
/// arbitrary witnesses (any two distinct fp32 values would do);
/// they are NOT tuned constants — the test asserts equality, not
/// anything about the magnitudes.
/// 2. Sanity: read both buffers back via mapped pinned and assert
/// they differ pointwise (the staged fill landed).
/// 3. Call `sync_target_from_online`.
/// 4. Synchronise the stream so the queued DtoD has retired.
/// 5. Read both buffers back via mapped pinned and assert
/// bit-for-bit equality across all `total_params` slots.
///
/// A stub body (e.g. `Ok(())` returning without copying) or a sync
/// against the wrong buffer / wrong direction / wrong stream all fail
/// step 5 — which the previous static `include_str!` regression test
/// could not detect.
#[test]
#[ignore = "gpu"]
fn iqn_sync_target_from_online_makes_target_equal_online() {
let ctx = CudaContext::new(0).expect("CUDA context — is GPU available?");
let stream = ctx.default_stream();
let shared = Arc::new(
PerStreamCublasHandles::new(&stream)
.expect("PerStreamCublasHandles::new"),
);
let config = GpuIqnConfig::default();
let mut iqn = GpuIqnHead::new(Arc::clone(&shared), config)
.expect("GpuIqnHead::new must succeed on a GPU-equipped host");
let n = iqn.total_params_for_test();
let head_stream = Arc::clone(iqn.stream_for_test());
// Witness values — arbitrary distinct fp32 constants. The test
// asserts post-sync equality, not anything about their magnitude.
const ONLINE_FILL: f32 = 0.42;
const TARGET_FILL: f32 = 0.99;
// Single mapped-pinned staging buffer reused across the two
// fills (one DtoD per fill, sync between so the staging memory
// can be safely overwritten).
let staging = unsafe { MappedF32Buffer::new(n) }
.expect("MappedF32Buffer alloc for fills");
fill_slice_via_mapped(&head_stream, &staging, iqn.online_params_slice(), n, ONLINE_FILL);
head_stream.synchronize().expect("stream sync after online fill");
fill_slice_via_mapped(&head_stream, &staging, iqn.target_params_slice(), n, TARGET_FILL);
head_stream.synchronize().expect("stream sync after target fill");
// Sanity: the fills actually landed and the buffers differ.
let online_pre = read_slice_via_mapped(&head_stream, iqn.online_params_slice(), n);
let target_pre = read_slice_via_mapped(&head_stream, iqn.target_params_slice(), n);
assert_eq!(online_pre.len(), n);
assert_eq!(target_pre.len(), n);
assert!(online_pre.iter().all(|&v| v == ONLINE_FILL),
"online fill did not propagate (saw {:?}…)", &online_pre[..online_pre.len().min(4)]);
assert!(target_pre.iter().all(|&v| v == TARGET_FILL),
"target fill did not propagate (saw {:?}…)", &target_pre[..target_pre.len().min(4)]);
assert_ne!(online_pre, target_pre,
"pre-sync buffers must differ — fill values were identical?");
// Exercise the contract under test.
iqn.sync_target_from_online()
.expect("sync_target_from_online must succeed");
head_stream.synchronize().expect("stream sync after sync_target_from_online");
// Read both back and assert bit-for-bit equality across the full
// parameter buffer. A stub body, wrong buffers, wrong direction,
// or queue against the wrong stream all fail this assertion.
let online_post = read_slice_via_mapped(&head_stream, iqn.online_params_slice(), n);
let target_post = read_slice_via_mapped(&head_stream, iqn.target_params_slice(), n);
// Online must be unchanged.
assert_eq!(online_post, online_pre,
"sync_target_from_online must not mutate online_params");
// Target must now equal online — exact bit equality (DtoD copy is
// not lossy; we use `f32::to_bits` to make NaN witnesses fail loud
// if the implementation ever changes).
assert_eq!(target_post.len(), online_post.len());
for (i, (t, o)) in target_post.iter().zip(online_post.iter()).enumerate() {
assert_eq!(t.to_bits(), o.to_bits(),
"target[{i}] != online[{i}] post-sync: target={t} online={o}");
}
}
}

View File

@@ -896,6 +896,10 @@ impl FusedTrainingCtx {
// producing oversized Adam steps in the first epochs whose effect
// compounds through the IQN backward pass into runaway gradients.
if let Some(iqn) = self.gpu_iqn.as_mut() {
// ┌─────────────────────────────────────────────────────────────┐
// │ DO NOT DELETE — fold-1 Q-drift regression trip-wire (#84) │
// └─────────────────────────────────────────────────────────────┘
//
// Hard-sync IQN target ← online at the same fold boundary as the
// main DQN's `sync_target_from_online` above. Without this, the
// IQN target buffer retains end-of-prior-fold weights while the
@@ -906,6 +910,18 @@ impl FusedTrainingCtx {
// atom support inside fold 1). Pairs with the IQN Adam reset
// immediately below — both are fold-boundary contract consumers
// that must migrate together (see `feedback_no_partial_refactor`).
//
// If you remove the `iqn.sync_target_from_online()` call below,
// fold-1 Q-drift returns and the model becomes unsafe for live
// trading (Kelly cap + runaway Q → oversized positions). The
// root-cause fix landed in commit `7c19b5903` (issue #84). A
// GPU runtime regression test exercises the contract end-to-
// end — see
// `cuda_pipeline::gpu_iqn_head::tests::iqn_sync_target_from_online_makes_target_equal_online`.
// It runs on the L40S smoke validation pool (carries
// `#[ignore = "gpu"]`); a stub body or a sync against the
// wrong buffer/stream fails the bit-for-bit equality assertion.
// Don't silence the test — restore the call.
if let Err(e) = iqn.sync_target_from_online() {
tracing::warn!("Fold-boundary IQN target sync failed (non-fatal): {e}");
} else {

View File

@@ -22,7 +22,8 @@ use common::metrics::{questdb_sink, training_metrics};
use tracing::{debug, info, warn};
use crate::cuda_pipeline::gpu_dqn_trainer::{
EPOCH_IDX_INDEX, EPSILON_EFF_INDEX, GAMMA_DIR_EFF_INDEX, TLOB_REGIME_FOCUS_EMA_INDEX,
EPOCH_IDX_INDEX, EPSILON_EFF_INDEX, GAMMA_DIR_EFF_INDEX, Q_ABS_REF_INDEX,
Q_DIR_ABS_REF_INDEX, TLOB_REGIME_FOCUS_EMA_INDEX,
};
use crate::dqn::logging::{log_epoch_start, log_epoch_end, log_training_progress};
use crate::dqn::target_update::convergence_half_life;
@@ -3925,20 +3926,51 @@ impl DQNTrainer {
// model can blow up a strategy. We must never deploy a model that
// exhibits this dynamic, so we halt training the moment we detect it.
//
// Criterion: if BOTH (a) |q_mean| has grown more than 2× since the
// previous epoch AND (b) |q_mean| is already past a 1.5 production-
// unsafe threshold, halt training and return error. The 2× threshold
// catches genuine geometric divergence (typical healthy growth is
// <30%/epoch); the 1.5 absolute floor prevents false positives in
// early training where small Q-magnitudes oscillate by large ratios
// (e.g. fold 0 ep4→ep5 shows 0.15→0.79 = 5.3× ratio but stays well
// inside production-safe range).
// Criterion: if BOTH (a) `|q_mean|` has grown more than 2× since the
// previous epoch AND (b) `|q_mean|` is already past an ISV-derived
// production-unsafe floor, halt training and return error.
//
// Both thresholds are numerical-stability bounds (Invariant 1 carve-
// out) — not tuned hyperparameters. They're derived from the bug
// signature observed in train-multi-seed-p526h: F1 ep2 Q=+2.23 (2.7×
// jump from F1 ep1 Q=+0.82) was the first epoch where divergence was
// unambiguous.
// * 2× ratio — architectural rate-of-change bound. Healthy training
// grows |q_mean| by <30%/epoch; a per-epoch doubling is geometric
// divergence regardless of absolute scale. Not a tuned constant.
//
// * Adaptive floor `kill_floor = 3.0 × max(ISV[Q_ABS_REF_INDEX],
// ISV[Q_DIR_ABS_REF_INDEX])` — both ISV slots track per-branch
// EMAs of `max(|Q_mean|)` over the magnitude (slot 16) and
// direction (slot 21) heads. The same EMAs already drive C51
// bin-weighting in `c51_loss_kernel.cu` and conviction shaping in
// `experience_kernels.cu` (search `q_abs_ref` / `q_dir_abs_ref`),
// so they reflect the model's *currently observed* healthy Q
// scale rather than a fixed deployment threshold. We take the
// `max()` across the two EMAs because either branch diverging
// past 3× its own normal scale is sufficient evidence of runaway
// — using min() would let one branch hide the other's drift.
//
// The `3.0×` multiplier is architectural: empirically RL Q-
// divergence shows up at roughly 2-4× normal scale on the same
// trajectory, with healthy oscillations staying inside ~1.5×.
// Per `feedback_isv_for_adaptive_bounds.md`, the *absolute level*
// comes from runtime ISV; the multiplier is the architectural
// "drift starts here" factor, not a tuned magnitude.
//
// * Bootstrap fallback: when neither ISV slot has graduated above
// `1e-6` (cold start, before the first stats-cadence write), the
// adaptive floor degenerates to 0. We then floor the bound at
// `0.5` so the kill criterion still has *some* lower bound during
// the first few epochs of fold 0. The `0.5` is a numerical-
// stability bound (Invariant 1 carve-out) chosen to sit safely
// below the ISV cold-start observed in healthy training (~0.05-
// 0.5 by epoch 3) — NOT a tuned trigger. Once ISV slots populate
// the formula uses runtime data alone.
//
// Both the 3.0× multiplier and the 0.5 cold-start floor are
// numerical-stability bounds (Invariant 1 carve-out), not tuned
// hyperparameters. The bug-signature trajectory that motivated the
// original 1.5 floor (train-multi-seed-p526h: F1 ep2 Q=+2.23, a 2.7×
// jump from F1 ep1 Q=+0.82) still trips this gate — the magnitude
// EMA ISV[16] would have been ~0.6 at that point (F1 first epoch
// |Q|≈0.82 with the standard α=0.05 EMA tracking), giving
// `kill_floor ≈ 3 × 0.6 = 1.8` and Q=+2.23 > 1.8 + ratio 2.7 trips.
//
// Skip on the first epoch of training (no prev_q to compare). DO NOT
// skip on fold-boundary epochs — fold-boundary divergence is
@@ -3947,14 +3979,33 @@ impl DQNTrainer {
let curr_abs = q_mean.abs();
let prev_abs = self.prev_epoch_q_mean.abs();
let ratio = curr_abs / prev_abs;
if curr_abs > 1.5 && ratio > 2.0 {
// Read both per-branch |Q| reference EMAs from the pinned/
// device-mapped ISV bus (zero-copy, mapped-pinned page — see
// `feedback_no_htod_htoh_only_mapped_pinned.md`). Take max() so
// either branch's healthy scale anchors the floor; using min()
// would let an under-active branch suppress drift on the other.
let (q_abs_ref_mag, q_abs_ref_dir) = if let Some(ref fused) = self.fused_ctx {
let mag = fused.trainer().read_isv_signal_at(Q_ABS_REF_INDEX).max(0.0);
let dir = fused.trainer().read_isv_signal_at(Q_DIR_ABS_REF_INDEX).max(0.0);
(mag, dir)
} else {
(0.0_f32, 0.0_f32)
};
let q_abs_ref_max = q_abs_ref_mag.max(q_abs_ref_dir);
// Cold-start floor (Invariant 1 carve-out): keep some bound
// before ISV EMAs graduate above the producer kernels' 1e-6
// epsilon. 0.5 sits safely below early-fold-0 healthy |Q|.
let kill_floor = (3.0_f64 * q_abs_ref_max as f64).max(0.5);
if curr_abs > kill_floor && ratio > 2.0 {
return Err(anyhow::anyhow!(
"Q-drift kill criterion triggered at epoch {}: \
|q_mean| jumped from {:.4} to {:.4} (ratio {:.2}×). \
|q_mean| jumped from {:.4} to {:.4} (ratio {:.2}×, \
adaptive floor {:.4} = max(0.5, 3× max(ISV[16]={:.4}, ISV[21]={:.4}))). \
A model with this dynamic is unsafe for production trading \
(Kelly cap → oversized positions). Halting training; \
model is rejected from deployment.",
epoch + 1, self.prev_epoch_q_mean, q_mean, ratio,
kill_floor, q_abs_ref_mag, q_abs_ref_dir,
));
}
}

View File

@@ -40,6 +40,58 @@ EMAs, spectral norm σ EMAs, TLOB Adam state. Touched: `training_loop.rs`
(+30 LOC). cargo check clean at 13 warnings (workspace baseline). No
fingerprint change.
Q-drift kill threshold made adaptive (2026-04-28, follow-up to commit
`1c917e3cb`): the absolute floor in the kill criterion (`curr_abs > 1.5`)
was a tuned constant in violation of `feedback_adaptive_not_tuned.md` and
`feedback_isv_for_adaptive_bounds.md`. Replaced with `kill_floor = max(0.5,
3.0 × max(ISV[Q_ABS_REF_INDEX=16], ISV[Q_DIR_ABS_REF_INDEX=21]))`. Both ISV
slots are per-branch EMAs of `max(|Q_mean|)` already maintained on-GPU by
`q_stats_kernel.cu` and consumed by `c51_loss_kernel`/`c51_grad_kernel`
for collapse-fraction normalisation; the kill criterion now anchors on
the same recently-observed healthy Q scale. The 3.0× multiplier is
architectural ("drift starts at ~2-4× healthy"), the 0.5 cold-start floor
is an Invariant-1 numerical-stability bound (active only while both ISV
slots are still ≤ 1e-6 in fold-0 epoch-1, then dominated by the ISV
formula). The 2× ratio gate is unchanged — it's already an architectural
rate-of-change bound. ISV reads use the existing pinned/device-mapped
path via `fused.trainer().read_isv_signal_at(...)` — no HtoD/DtoH per
`feedback_no_htod_htoh_only_mapped_pinned.md`. Touched: `training_loop.rs`
(+~70 LOC, two new use-list imports). cargo check clean at 13 warnings.
No fingerprint change.
IQN fold-boundary sync GPU regression test (2026-04-28, follow-up to
commit `7c19b5903`, resolves issue #84): added
`iqn_sync_target_from_online_makes_target_equal_online` in
`cuda_pipeline/gpu_iqn_head.rs::tests`. The test fills
`online_params` ← 0.42 and `target_params` ← 0.99 via mapped-pinned
staging + `cuMemcpyDtoDAsync` (no HtoD per
`feedback_no_htod_htoh_only_mapped_pinned.md`), sanity-checks the
buffers differ, calls `sync_target_from_online`, synchronises the
stream, reads both buffers back through fresh `MappedF32Buffer`
allocations, and asserts bit-for-bit equality across all
`total_params` slots (using `f32::to_bits` so any future NaN-bearing
implementation also fails loud). The witnesses 0.42 / 0.99 are
arbitrary distinct fp32 constants, not tuned values — the contract
asserted is equality of the buffers, independent of magnitude. Carries
`#[ignore = "gpu"]` and runs on the L40S smoke validation pool;
`cargo test -p ml --lib` on a CPU-only host skips it. Replaces the
earlier static-source `include_str!` guard which only caught literal
deletion of the call line — a stub body returning `Ok(())`, a copy
against the wrong buffer / wrong direction, or a queue against the
wrong stream all silently passed the static guard but now fail the
runtime equality assertion. Buffer access is exposed through three
new `#[cfg(test)] pub(crate)` accessors on `GpuIqnHead`
(`online_params_slice`, `target_params_slice`, `total_params_for_test`,
`stream_for_test`) so the public API is not widened. Paired with a
strengthened doc-block at the call site in `fused_training.rs` (boxed
`DO NOT DELETE` warning + reference to the new test name and issue
#84) so anyone touching the line sees the regression context inline
before deleting. Touched: `gpu_iqn_head.rs` (4 cfg(test) accessors +
new tests mod with helpers + the runtime test, ~+200 LOC),
`fused_training.rs` (boxed comment expansion, ~+15 LOC, no behaviour
change). cargo check clean at 13 warnings (workspace baseline). No
fingerprint change.
Fold-boundary reset gap — IQN readiness gauge (2026-04-28, Fix 3 of
3): the IQN readiness gauge on `GpuDqnTrainer` is a streaming
improvement fraction `iqn_readiness = (iqn_loss_initial -