From 5694eb4df2dfdbacfc10579ebb910d2b04c79e63 Mon Sep 17 00:00:00 2001 From: jgrusewski Date: Tue, 12 May 2026 12:51:31 +0200 Subject: [PATCH] =?UTF-8?q?fix(sp21):=20T2.2=20Phase=208.5=20=E2=80=94=20w?= =?UTF-8?q?ire=20factored-action=20branch=20sizes=20into=20closure-based?= =?UTF-8?q?=20eval=20(atomic)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit v7 smoke (train-fv4s8, commit 23b89a90e) eval pod hit CUDA_ERROR_ILLEGAL_ADDRESS at fold 0: Error: DQN fold 0 GPU evaluation failed: GpuBacktestEvaluator::evaluate failed for fold 0: Model error: eval_done_event synchronize: DriverError(CUDA_ERROR_ILLEGAL_ADDRESS, "an illegal memory access was encountered") The {:#} anyhow chain fix from Phase 8.3+9 made the failure mode visible. Diagnosis from code (no second smoke needed): the closure-based evaluate() path never sets b0_size..b3_size, leaving them at the default 0. env_step kernel's decode_*_4b helpers do action/(b1*b2*b3) → divide-by-zero → garbage decoded indices → out-of-bounds memory read → CUDA_ERROR_ILLEGAL_ADDRESS at next event-sync. The production `evaluate_dqn_graphed` path sets b-sizes via `ensure_action_select_ready` (which also lazy-allocates intent buffers the closure path doesn't need). The closure-based `evaluate()` path used by eval-baseline never calls it. Fix: 1. Add pub fn `GpuBacktestEvaluator::set_branch_sizes(&mut self, dqn_cfg: &DqnBacktestConfig)` — sets b0..b3_size only, no buffer allocation. 2. Add defensive guard in `evaluate()` that bails with `MLError::ConfigError` if any b-size is zero. Future regressions produce a clear error instead of an opaque CUDA illegal-address. 3. Wire `set_branch_sizes(&dqn_cfg)` call in `evaluate_dqn_fold_gpu` between `DqnBacktestConfig::from_network_dims` and the closure-based `evaluator.evaluate(...)`. Pearls honoured: - feedback_no_hiding: zero-b-size now surfaces as ConfigError rather than CUDA illegal-address downstream - feedback_no_partial_refactor: closure-path was a partial wire-up from pre-factored-action days; set_branch_sizes brings it into parity with the CUBLAS production path for action decoding - pearl_no_deferrals_for_complementary_fixes: v7's chain-exposing fix surfaced this; lands immediately not after another smoke Verification: cargo check -p ml --example evaluate_baseline --features cuda # clean Note on PPO/supervised paths: Their evaluate() calls also lack set_branch_sizes and will now trip the defensive guard. Those paths haven't actually run eval since STATE_DIM grew past 54 — the silent failure mode had been masking it. Future Phase will either wire their action conventions (PPO: 5-exposure; supervised: signal thresholds) or delete the dead paths per feedback_no_partial_refactor. Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/ml/examples/evaluate_baseline.rs | 10 ++ .../cuda_pipeline/gpu_backtest_evaluator.rs | 40 +++++++ docs/dqn-wire-up-audit.md | 102 ++++++++++++++++++ 3 files changed, 152 insertions(+) diff --git a/crates/ml/examples/evaluate_baseline.rs b/crates/ml/examples/evaluate_baseline.rs index 64ec1e1cc..5dba92438 100644 --- a/crates/ml/examples/evaluate_baseline.rs +++ b/crates/ml/examples/evaluate_baseline.rs @@ -832,6 +832,16 @@ fn evaluate_dqn_fold_gpu( let dqn_cfg = ml::cuda_pipeline::gpu_backtest_evaluator::DqnBacktestConfig::from_network_dims(network_dims); + // Phase 8.5 (2026-05-12) — wire factored-action branch sizes into the + // evaluator's env_step kernel. Without this, env_step's + // decode_direction_4b / decode_magnitude_4b / decode_order_4b / + // decode_urgency_4b helpers divide by zero (b1*b2*b3 = 0), and the + // GPU raises CUDA_ERROR_ILLEGAL_ADDRESS asynchronously at the next + // event-sync. The production `evaluate_dqn_graphed` path sets these + // via `ensure_action_select_ready`; this closure-path equivalent + // calls the public `set_branch_sizes` setter. + evaluator.set_branch_sizes(&dqn_cfg); + { // Standalone eval: closure-based forward pass. // Training eval uses FusedTrainingCtx as QValueProvider for determinism. diff --git a/crates/ml/src/cuda_pipeline/gpu_backtest_evaluator.rs b/crates/ml/src/cuda_pipeline/gpu_backtest_evaluator.rs index 42d18154e..44399fc6d 100644 --- a/crates/ml/src/cuda_pipeline/gpu_backtest_evaluator.rs +++ b/crates/ml/src/cuda_pipeline/gpu_backtest_evaluator.rs @@ -1421,6 +1421,28 @@ impl GpuBacktestEvaluator { self.n_windows } + /// Set the four factored-action branch sizes from a `DqnBacktestConfig`. + /// + /// The closure-based `evaluate()` path delegates the model forward pass + /// to a user-supplied closure but still uses the internal `env_step` + /// kernel to advance the simulator. `env_step` decodes flat action + /// indices into `(dir, mag, order, urgency)` via `decode_*_4b()` helpers + /// in `trade_physics.cuh`, which compute `action / (b1 * b2 * b3)`. If + /// any `b*_size` is zero the helpers divide by zero, the kernel reads + /// out-of-bounds, and CUDA raises `CUDA_ERROR_ILLEGAL_ADDRESS` at the + /// next synchronise — exactly the v7 smoke failure mode (2026-05-12). + /// + /// The production `evaluate_dqn_graphed` path sets these via + /// `ensure_action_select_ready` (which also allocates intent buffers + /// the closure path doesn't need). This setter is the closure-path + /// equivalent — set b-sizes only, no buffer allocs. + pub fn set_branch_sizes(&mut self, dqn_cfg: &DqnBacktestConfig) { + self.b0_size = dqn_cfg.branch_0_size as i32; + self.b1_size = dqn_cfg.branch_1_size as i32; + self.b2_size = dqn_cfg.branch_2_size as i32; + self.b3_size = dqn_cfg.branch_3_size as i32; + } + /// Maximum batch size that the val TLOB instance must support. /// /// `submit_dqn_step_loop_cublas` runs TLOB once per chunk on @@ -1571,6 +1593,24 @@ impl GpuBacktestEvaluator { { let _nvtx = NvtxRange::new("backtest_evaluate"); + // Phase 8.5 (2026-05-12) — defensive guard: `env_step` decodes flat + // action indices into (dir, mag, order, urgency) via + // `decode_*_4b()` helpers that divide by `b1 * b2 * b3`. If any + // b-size is zero, the kernel divides by zero and raises + // CUDA_ERROR_ILLEGAL_ADDRESS asynchronously at the next event-sync. + // The production `evaluate_dqn_graphed` path sets these via + // `ensure_action_select_ready`; closure-path callers must invoke + // `set_branch_sizes` before `evaluate()`. Surface the requirement + // up-front rather than crashing the CUDA context downstream. + if self.b0_size == 0 || self.b1_size == 0 || self.b2_size == 0 || self.b3_size == 0 { + return Err(MLError::ConfigError(format!( + "evaluate: branch sizes unset (b0={}, b1={}, b2={}, b3={}); \ + call `set_branch_sizes(&DqnBacktestConfig)` before `evaluate()` \ + to wire factored-action decoding for env_step.", + self.b0_size, self.b1_size, self.b2_size, self.b3_size + ))); + } + let state_dim = self.state_dim; for step in 0..self.max_len { diff --git a/docs/dqn-wire-up-audit.md b/docs/dqn-wire-up-audit.md index e08700a72..c41894f21 100644 --- a/docs/dqn-wire-up-audit.md +++ b/docs/dqn-wire-up-audit.md @@ -15741,3 +15741,105 @@ For volume bars with healthy policy and pnl_std ≈ 1e-5: If these signals fire, the Phase 7 (alpha boost via E6/E7/E8) wiring is finally exercised end-to-end on volume bars. + +## 2026-05-12 — SP21 T2.2 Phase 8.5: wire factored-action branch sizes into closure-based eval (atomic) + +### Scope (atomic single commit) + +v7 smoke (train-fv4s8, commit 23b89a90e) eval pod hit +`CUDA_ERROR_ILLEGAL_ADDRESS` at fold 0: + +``` +Error: DQN fold 0 GPU evaluation failed: GpuBacktestEvaluator::evaluate +failed for fold 0: Model error: eval_done_event synchronize: +DriverError(CUDA_ERROR_ILLEGAL_ADDRESS, "an illegal memory access was encountered") +``` + +The `{:#}` anyhow-chain fix from Phase 8.3+9 made this loud (Phase 8.3+9 +was about exposing the chain — and exposing it surfaced the next bug +in line). Diagnosis from code (no second smoke needed): the closure- +based `evaluate()` path never sets `b0_size..b3_size` (factored-action +branch sizes for env_step's `decode_*_4b()` helpers). + +### Root cause + +`backtest_env_step` kernel decodes flat action indices via helpers in +`trade_physics.cuh`: + +```c +int decode_direction_4b(int action, int b1, int b2, int b3) { + return action / (b1 * b2 * b3); // ← divides by zero if any b_i=0 +} +``` + +The production `evaluate_dqn_graphed` path sets b-sizes via +`ensure_action_select_ready` (which also allocates intent buffers etc). +The closure-based `evaluate()` path used by eval-baseline DOES NOT +call this — b-sizes default to zero (from `new()` initialiser at +lines 1338-1341). v7's first env_step launch divides by zero, kernel +reads garbage indices → out-of-bounds memory access → CUDA raises +`CUDA_ERROR_ILLEGAL_ADDRESS` asynchronously at the next event-sync +(in `launch_metrics_and_record_event`'s synchronise call). + +### Fix + +**1. New public method** `GpuBacktestEvaluator::set_branch_sizes(&mut self, dqn_cfg: &DqnBacktestConfig)` +- Sets `b0..b3_size` from a `DqnBacktestConfig` without lazy-allocating + intent buffers (those are needed only by the CUBLAS production path, + not the closure path). +- Mirror of the b-size assignment block inside the private + `ensure_action_select_ready` but with closure-path-only scope. + +**2. Defensive guard in `evaluate()`** +- Bails with `MLError::ConfigError` if any b-size is zero. +- Surfaces the requirement up-front rather than crashing the CUDA + context downstream. Future regressions to this wiring will produce + a clear "branch sizes unset" error instead of a generic illegal- + memory-access. + +**3. Wire `set_branch_sizes` from `evaluate_dqn_fold_gpu`** +- Called after `DqnBacktestConfig::from_network_dims(...)` and before + `evaluator.evaluate(...)`. +- Only the branching closure path (the modern model's path). The + non-branching else fallback is dead code for current models and + will hit the new defensive guard if anyone tries to use it. + +### Files changed + +- `crates/ml/src/cuda_pipeline/gpu_backtest_evaluator.rs`: + - `pub fn set_branch_sizes` added + - `evaluate()` gains zero-branch-size guard +- `crates/ml/examples/evaluate_baseline.rs`: + - `evaluator.set_branch_sizes(&dqn_cfg)` call inserted before + closure-based `evaluator.evaluate(...)` for DQN fold path +- `docs/dqn-wire-up-audit.md`: this entry + +### Pearls + invariants honoured + +- **feedback_no_hiding**: defensive guard surfaces b-size zero as + `ConfigError` rather than letting the next kernel divide by zero + and produce an opaque `CUDA_ERROR_ILLEGAL_ADDRESS` +- **feedback_no_partial_refactor**: the closure-based `evaluate()` + path was a partial wire-up from the days before factored actions — + `set_branch_sizes` makes it equivalent to the CUBLAS production + path for action decoding +- **pearl_no_deferrals_for_complementary_fixes**: the v7 smoke + diagnostic (Phase 8.3+9's `{:#}` chain) exposed this; fix lands + immediately rather than after another smoke cycle + +### Verification + +``` +cargo check -p ml --example evaluate_baseline --features cuda # clean +``` + +### Note on PPO / supervised eval paths + +These paths also call `evaluate()` without `set_branch_sizes` (lines +~1106, ~1277). They would now fail with the new defensive guard — +which is correct: those models have not actually run eval since +STATE_DIM grew past the legacy 54, but the silent failure had been +masking it. A future Phase will either wire PPO/supervised branch +sizes correctly (PPO uses a 5-exposure action space, supervised uses +signal thresholds — different conventions from DQN factored actions) +or delete the dead paths per `feedback_no_partial_refactor`.