diff --git a/crates/ml/src/trainers/dqn/smoke_tests/performance.rs b/crates/ml/src/trainers/dqn/smoke_tests/performance.rs index 984c5d5d7..8313479a7 100644 --- a/crates/ml/src/trainers/dqn/smoke_tests/performance.rs +++ b/crates/ml/src/trainers/dqn/smoke_tests/performance.rs @@ -20,6 +20,12 @@ fn data_dir() -> String { } /// Measure end-to-end training throughput (3 epochs, real data). +/// +/// Purpose: verify the training pipeline runs at non-trivial speed AND produces +/// plausible metrics. Previously only checked `loss.is_finite()` which passes on +/// trivial zeros or NaN-less garbage — giving no signal that anything healthy +/// happened. We now assert a conservative steps/second floor and bound the +/// Q-value magnitude. #[test] #[ignore] // Loads real training data — run via nightly CI or manual trigger fn test_training_throughput_measurement() -> anyhow::Result<()> { @@ -34,16 +40,62 @@ fn test_training_throughput_measurement() -> anyhow::Result<()> { }))?; let elapsed = start.elapsed(); + // ── loss: finite and non-negative ── assert_finite(metrics.loss, "loss"); + assert!( + metrics.loss >= 0.0, + "loss should be non-negative (sign-bug indicator), got {}", + metrics.loss, + ); + + // ── epochs must have run ── + assert!( + metrics.epochs_trained >= 1, + "no epoch completed — throughput cannot be measured. metrics: {:?}", + metrics, + ); + + // ── throughput floor ── + // RTX 3050 Ti smoketest target: > 10 epochs/s is aspirational, > 0.05 epochs/s + // (i.e. each epoch < 20s) is a safe local floor. Raise in CI if desired — the + // point of this floor is to catch catastrophic regressions (e.g. accidentally + // falling back to CPU, or a kernel that ran but pinned a CPU loop). + let epochs_per_sec = metrics.epochs_trained as f64 / elapsed.as_secs_f64().max(1e-6); + assert!( + epochs_per_sec > 0.05, + "Training throughput {:.3} epochs/s below floor 0.05 (laptop target). \ + {} epochs in {:.2}s — CI may set a higher floor.", + epochs_per_sec, + metrics.epochs_trained, + elapsed.as_secs_f64(), + ); + + // ── avg_q_value must be present, finite, and bounded ── + // Rules out NaN-like explosions that happen to be finite-but-huge. + let avg_q = metrics + .additional_metrics + .get("avg_q_value") + .copied() + .unwrap_or(f64::NAN); + assert!( + avg_q.is_finite(), + "avg_q_value missing or NaN — diagnostic regressed: {:?}", + metrics.additional_metrics.get("avg_q_value"), + ); + assert!( + avg_q.abs() < 1e6, + "avg_q_value {avg_q} is implausibly large (C51 atoms exhausted or \ + NaN-like explosion). v_range is finite by construction.", + ); + info!( elapsed_secs = elapsed.as_secs_f64(), epochs_trained = metrics.epochs_trained, + epochs_per_sec = epochs_per_sec, "Training throughput" ); info!(loss = metrics.loss, "Avg loss"); - if let Some(&avg_q) = metrics.additional_metrics.get("avg_q_value") { - info!(avg_q_value = avg_q, "Avg Q-value"); - } + info!(avg_q_value = avg_q, "Avg Q-value"); if let Some(&final_eps) = metrics.additional_metrics.get("final_epsilon") { info!(final_epsilon = final_eps, "Final epsilon"); } @@ -121,6 +173,12 @@ fn test_per_sample_latency() -> anyhow::Result<()> { /// Single-epoch training on real data. /// Loads full 163K-bar dataset — too slow for CI (run via nightly or manual trigger). +/// +/// Purpose: one epoch on real ES.FUT data produces valid training metrics. +/// Previously asserted only `loss.is_finite()`, which passes on `loss == 0.0` +/// (a sign-bug / accumulation-bug tell) or any bounded NaN-less value. A +/// healthy epoch must complete at least one step, produce bounded non-negative +/// loss, and a bounded finite avg_q_value. #[test] #[ignore] fn test_real_data_single_epoch() -> anyhow::Result<()> { @@ -137,15 +195,56 @@ fn test_real_data_single_epoch() -> anyhow::Result<()> { }))?; let elapsed = start.elapsed(); + // ── loss: finite, bounded, non-negative ── + // Negative loss would signal a sign bug (loss is ≥0 by construction — MSE + C51 + // KL + CQL are all non-negative). Zero loss on a real DQN training run is also + // a tell: either the network is predicting exactly the targets (impossible at + // step 1) or the backward pass is returning nothing (accumulation bug). The + // upper bound 1e8 rules out NaN-propagation disguised as "finite". assert_finite(metrics.loss, "real_data_loss"); + assert!( + metrics.loss >= 0.0, + "real_data loss is negative ({}) — sign bug in MSE / C51 / CQL accumulator", + metrics.loss, + ); + assert!( + metrics.loss < 1e8, + "real_data loss {} exceeds bound 1e8 — finite but implausibly huge, check \ + NaN propagation or reward scaling", + metrics.loss, + ); + + // ── at least one epoch completed ── + assert!( + metrics.epochs_trained >= 1, + "real_data single-epoch test produced 0 epochs. metrics: {:?}", + metrics, + ); + + // ── avg_q_value: finite + bounded ── + let avg_q = metrics + .additional_metrics + .get("avg_q_value") + .copied() + .unwrap_or(f64::NAN); + assert!( + avg_q.is_finite(), + "real_data avg_q_value missing/NaN: {:?}", + metrics.additional_metrics.get("avg_q_value"), + ); + assert!( + avg_q.abs() < 1e6, + "real_data avg_q_value {avg_q} is implausibly large — C51 atoms exhausted \ + or NaN-like explosion", + ); + info!( elapsed_secs = elapsed.as_secs_f64(), loss = metrics.loss, + epochs_trained = metrics.epochs_trained, + avg_q_value = avg_q, "Real-data single epoch" ); - if let Some(&avg_q) = metrics.additional_metrics.get("avg_q_value") { - info!(avg_q_value = avg_q, "Avg Q-value"); - } drop(trainer); drop(rt); Ok(())