test(iqn-sync): real GPU runtime test for sync_target_from_online (replaces static test)
Replace the proposed static `include_str!` regression guard for the fold-boundary IQN target hard-sync (issue #84, root-cause fix in commit `7c19b5903`) with a real GPU runtime test that exercises the contract end-to-end. The static guard only caught literal deletion of the call line — a stub body returning `Ok(())`, a copy against the wrong buffer, the wrong copy direction, or a queue against the wrong stream all pass the textual assertion silently. Test (`cuda_pipeline::gpu_iqn_head::tests::iqn_sync_target_from_online_makes_target_equal_online`): 1. Construct a `GpuIqnHead` with default `GpuIqnConfig` on the default CUDA stream. 2. Fill `online_params` ← 0.42 and `target_params` ← 0.99 via a single mapped-pinned staging buffer + `cuMemcpyDtoDAsync`. No HtoD copy is issued; the host write through `MappedF32Buffer::host_ptr` reaches the GPU through the device-mapped alias and the DtoD copies the staged values into each parameter buffer. The witnesses 0.42 / 0.99 are arbitrary distinct fp32 constants — the contract asserted is buffer equality, independent of magnitude. 3. Sanity: read both buffers back via fresh mapped-pinned destinations + DtoD, assert they differ pointwise. 4. Call `iqn.sync_target_from_online()`. 5. `stream.synchronize()` so the queued DtoD has retired. 6. Read both buffers back and assert bit-for-bit equality across all `total_params` slots using `f32::to_bits` (so any future NaN-bearing implementation also fails loud). Per `feedback_no_htod_htoh_only_mapped_pinned.md`, all CPU↔GPU communication routes through `cuMemHostAlloc(DEVICEMAP|PORTABLE)` mapped pinned memory. Tests are not exempt — fills and read-backs both use `MappedF32Buffer` + `cuMemcpyDtoDAsync`. Buffer access exposed via four 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. Test carries `#[ignore = "gpu"]` matching the smoke-test convention already used in `regression_detection.rs`. `cargo test -p ml --lib` on a CPU-only host (the worktree environment) skips it cleanly; the L40S smoke validation pool runs it via `--ignored`. Paired with a strengthened doc-block at the call site in `fused_training.rs::reset_for_fold` (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 + tests mod with helpers + the runtime test, +217 LOC), `fused_training.rs` (boxed comment + test reference, +16 LOC, no behaviour change), `docs/dqn-wire-up-audit.md` (audit entry replacing the static-test entry from the previous proposal, +33 LOC). Verified: * `cargo check -p ml --lib` — clean at 13 warnings (workspace baseline). * `cargo test -p ml --lib --no-run` — clean at 24 warnings (test profile baseline). * `cargo test -p ml --lib state_reset_registry` — 3/3 existing tests pass (no 4th static-source test added). * `cargo test -p ml --lib gpu_iqn_head` — 1 test discovered, correctly reports `ignored, gpu` on this CPU-only worktree. Local run not attempted — worktree environment lacks a GPU. The test runs as part of L40S smoke validation via `--ignored`. No fingerprint change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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}");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -59,6 +59,39 @@ path via `fused.trainer().read_isv_signal_at(...)` — no HtoD/DtoH per
|
||||
(+~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 -
|
||||
|
||||
Reference in New Issue
Block a user