From 71eab9a253c2aecb2e26864b602aee5ee54706fa Mon Sep 17 00:00:00 2001 From: jgrusewski Date: Tue, 12 May 2026 22:51:20 +0200 Subject: [PATCH] =?UTF-8?q?fix(tests):=20gpu=5Fbacktest=5Fvalidation=20act?= =?UTF-8?q?ion=20constants=20=E2=80=94=20Long100=20is=2072,=20not=204?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two `gpu_backtest_validation` tests were failing with bit-identical deterministic values for 2+ months: `test_always_long_on_downtrend` (expected negative PnL, got +0.00023627281) and `test_multiple_windows_produce_results` (uptrend < downtrend instead of > ). Root cause: SP21 Phase 8.5 (2026-05-12) wired the factored 4-3-3-3 action decoder into the eval (`backtest_state_gather` + env_step), but the test's hardcoded `constant_action_model(4, ...)` integer literal wasn't migrated. Pre-Phase-8.5 the eval used a flat 4-action enum where `4` reportedly meant Long100; the factored decoder now interprets `4` as: decode_direction_4b(4, b1=3, b2=3, b3=3) = 4 / 27 = 0 = DIR_SHORT decode_magnitude_4b(4, b1=3, b2=3, b3=3) = (4/9) % 3 = 0 = MAG_QUARTER So the test was running Short-Quarter (-0.25 position) on the trend fixtures. On random-walk synthetic prices with drift ±0.001 vs σ=0.01 noise per bar, the 24-step eval window has S/N ≈ 0.49 — specific seeds can produce net-against-drift trajectories, making the actual short-quarter PnL small but deterministic, with sign flipped relative to test intent. Fix: change `constant_action_model(4, ...)` → `constant_action_model(72, ...)` in the 2 failing tests. Action 72 = dir=LONG (2) * 27 + mag=FULL (2) * 9 + 0 + 0 — the actual "Long100" under 4-3-3-3 factoring. Both tests now pass; no regressions on the 4 previously-passing tests. Verification ──────────── - gpu_backtest_validation pre-fix: 4 passed, 2 failed - gpu_backtest_validation post-fix: 6 passed, 0 failed Out of scope for this commit (follow-up audit needed) ───────────────────────────────────────────────────── Three other tests in the same file have the same stale `4` constant with misleading "Always Long100" comments, but currently pass incidentally: - `test_always_long_on_uptrend` (line 218): asserts `total_pnl > 0`. Currently passes BY ACCIDENT — action=4 (Short-Quarter) on seed-42's net-down 24-bar trajectory produces +PnL, satisfying the assertion for the wrong reason. Fixing to action=72 alone would break this test (true Long100 on seed-42's net-down trajectory is negative); the test needs BOTH the action fix AND a seed/window change so the "uptrend" trajectory actually trends up over the eval window (e.g., 250-bar window or drift=0.01). - `test_extended_metrics_populated` (line 465) and `test_active_model_records_trades` (line 524): assertions are direction-agnostic (VaR/CVaR/Calmar/Omega NaN-check + CVaR≤VaR; total_trades > 0 + win_rate range), so they pass legitimately under whatever-direction action=4 produces. The "Long100" comments are misleading but the tests are correctly covering their stated behavior. A separate audit-and-fix pass should address all three at once: either correct the action constants + adjust seed/window to ensure each test's named trajectory direction is statistically reliable, or introduce a named constant (e.g., LONG100_ACTION) and helper to prevent the same drift recurring. Refs ──── - SP21 T2.2 Phase 8.5 commit 5694eb4df: "wire factored-action branch sizes into closure-based eval (atomic)" - crates/ml/src/cuda_pipeline/trade_physics.cuh: `decode_direction_4b`, `decode_magnitude_4b` — canonical factored action decoders - crates/ml/src/cuda_pipeline/gpu_backtest_evaluator.rs:189: `DqnBacktestConfig::from_network_dims` — sets branch_sizes (4,3,3,3) Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/ml/tests/gpu_backtest_validation.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/crates/ml/tests/gpu_backtest_validation.rs b/crates/ml/tests/gpu_backtest_validation.rs index ebdef2932..2853d2d0b 100644 --- a/crates/ml/tests/gpu_backtest_validation.rs +++ b/crates/ml/tests/gpu_backtest_validation.rs @@ -284,7 +284,14 @@ mod gpu_tests { // env_step decoder. Match production eval-baseline.rs. evaluator.set_branch_sizes(&DqnBacktestConfig::from_network_dims((256, 256, 128, 128))); - let model = constant_action_model(4, &stream); // Always Long100 + // Action 72 = Long×Full×order=0×urgency=0 under 4-3-3-3 factored + // decoding (action = dir*27 + mag*9 + order*3 + urgency = 2*27 + + // 2*9 + 0 + 0). Pre-Phase-8.5 the eval used a flat 4-action enum + // where `4` reportedly meant Long100, but SP21 Phase 8.5 + // (2026-05-12) wired the factored decoder where `4` now decodes + // to Short-Quarter (dir=0, mag=0). The "Always Long100" comment + // was correct intent but the constant wasn't migrated. + let model = constant_action_model(72, &stream); // Always Long100 (factored) let metrics = evaluator .evaluate(&model, 24) .expect("evaluation should succeed"); @@ -405,7 +412,9 @@ mod gpu_tests { // env_step decoder. Match production eval-baseline.rs. evaluator.set_branch_sizes(&DqnBacktestConfig::from_network_dims((256, 256, 128, 128))); - let model = constant_action_model(4, &stream); // Always Long100 + // Action 72 = Long×Full×order=0×urgency=0 under 4-3-3-3 factored + // decoding (see test_always_long_on_downtrend for full derivation). + let model = constant_action_model(72, &stream); // Always Long100 (factored) let metrics = evaluator .evaluate(&model, 24) .expect("evaluation should succeed");