diff --git a/docs/superpowers/plans/2026-03-26-final-fixes-before-h100.md b/docs/superpowers/plans/2026-03-26-final-fixes-before-h100.md new file mode 100644 index 000000000..852bcbb51 --- /dev/null +++ b/docs/superpowers/plans/2026-03-26-final-fixes-before-h100.md @@ -0,0 +1,315 @@ +# Final Fixes Before H100 — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Fix all 6 remaining issues blocking H100 deployment: train/eval mismatch, NaN objective, broken trial budget, hardcoded CVaR threshold, duplicate trade threshold, stale test vectors. + +**Architecture:** The root cause chain is: broken trial budget → too many evals → backtest without hold enforcement → NaN Sharpe → 1e6 penalty. Fix from bottom up: (1) add hold_time tracking to backtest kernel, (2) wire hold enforcement into evaluator's action_select, (3) fix trial budget observer, (4) make remaining hardcoded values dynamic. + +**Tech Stack:** CUDA kernel (C), Rust (cudarc 0.19.3), ml-hyperopt crate. + +--- + +## Task 1: Add hold_time tracking to backtest_env_kernel.cu + +**Files:** +- Modify: `crates/ml/src/cuda_pipeline/backtest_env_kernel.cu` +- Modify: `crates/ml/src/cuda_pipeline/gpu_backtest_evaluator.rs` + +The backtest kernel's `portfolio_buf` is stride-8 (8 floats/window). The `experience_action_select` kernel reads `ps[10]` (hold_time) at stride-20. These are incompatible. The simplest fix: add `hold_time` as field [5] in the backtest portfolio (currently `[5]`, `[6]`, `[7]` are unused/zero). + +- [ ] **Step 1: Add hold_time tracking to backtest_env_step kernel** + +In `backtest_env_kernel.cu`, find the portfolio state layout. Currently: +``` +[0] value, [1] position, [2] cash, [3] unrealised_pnl, [4] max_equity, [5-7] unused +``` + +Add hold_time tracking at index [5]: +```cuda +// Read hold_time from portfolio state +float hold_time = portfolio[w * 8 + 5]; + +// After position update, track hold_time +if (fabsf(new_position) > 0.001f && fabsf(prev_position) > 0.001f) { + // Same direction — increment hold + int prev_s = (prev_position > 0.001f) ? 1 : -1; + int curr_s = (new_position > 0.001f) ? 1 : -1; + if (prev_s == curr_s) { + hold_time += 1.0f; + } else { + hold_time = 1.0f; // Reversal — new trade starts + } +} else if (fabsf(new_position) > 0.001f && fabsf(prev_position) < 0.001f) { + hold_time = 1.0f; // Entry from flat +} else { + hold_time = 0.0f; // Flat +} + +// Write back +portfolio[w * 8 + 5] = hold_time; +``` + +- [ ] **Step 2: Add min_hold_bars hold enforcement to backtest_env_step** + +Add `int min_hold_bars` as a kernel parameter. After computing new_position but before applying it: + +```cuda +// Hold enforcement (Layer 2 — same as training kernel) +int prev_sign = (prev_position > 0.001f) ? 1 : ((prev_position < -0.001f) ? -1 : 0); +int curr_sign = (new_position > 0.001f) ? 1 : ((new_position < -0.001f) ? -1 : 0); +int wants_exit = (prev_sign != 0 && curr_sign == 0); +int wants_reversal = (prev_sign != 0 && curr_sign != 0 && prev_sign != curr_sign); + +if (hold_time > 0.0f && hold_time < (float)min_hold_bars + && (wants_exit || wants_reversal) + && step < window_len - 1) { + new_position = prev_position; // Override: keep position +} +``` + +- [ ] **Step 3: Pass min_hold_bars from Rust** + +In `gpu_backtest_evaluator.rs`: +- Add `min_hold_bars: i32` to `GpuBacktestConfig` (default: 5) +- Pass to `backtest_env_step` kernel launch as the last arg +- Wire from hyperopt adapter: `config.min_hold_bars = hyperparams.min_hold_bars as i32` + +- [ ] **Step 4: Build + test** + +Run: `SQLX_OFFLINE=true cargo check -p ml --lib` +Run: `SQLX_OFFLINE=true cargo test -p ml --lib -- backtest` + +- [ ] **Step 5: Commit** + +```bash +git commit -m "feat: hold_time tracking + hold enforcement in backtest_env_step + +Backtest portfolio now tracks hold_time at index [5]. min_hold_bars +param enforces hold constraint during evaluation, matching training. +Fixes train/eval mismatch that caused NaN Sharpe → 1e6 penalty." +``` + +--- + +## Task 2: Wire hold enforcement into evaluator's action_select + +**Files:** +- Modify: `crates/ml/src/cuda_pipeline/gpu_backtest_evaluator.rs` + +Now that the backtest kernel tracks hold_time at `portfolio[w*8+5]`, we need a stride-20 adapter buffer for `experience_action_select`. But actually, since the evaluator uses greedy argmax (epsilon=0), the action masking is simpler — we can just let the backtest_env_step hold enforcement (Layer 2 from Task 1) handle it. The action_select kernel doesn't NEED hold masking during eval because: +1. Epsilon = 0 (greedy, no random exploration to mask) +2. The env_step kernel overrides any hold-violating action anyway + +So we keep `null_portfolio/0/0` in the action_select launch but rely on the backtest_env_step hold enforcement from Task 1 to enforce holds. This avoids the stride-8/stride-20 incompatibility. + +- [ ] **Step 1: Update the comment to document the design decision** + +Replace the comment at the evaluator's action_select launch: +```rust +let null_portfolio: u64 = 0; // Hold enforcement handled by backtest_env_step (Layer 2) +let eval_min_hold: i32 = 0; // Action masking disabled — greedy argmax + env override sufficient +let eval_max_pos: f32 = 0.0; +``` + +- [ ] **Step 2: Build + test** + +Run: `SQLX_OFFLINE=true cargo check -p ml --lib` + +- [ ] **Step 3: Commit** + +```bash +git commit -m "docs: clarify evaluator action masking strategy — Layer 2 in env_step handles holds" +``` + +--- + +## Task 3: Fix trial budget observer + +**Files:** +- Modify: `crates/ml-hyperopt/src/observer.rs` +- Modify: `crates/ml-hyperopt/src/optimizer.rs` + +The `TrialBudgetObserver.increment_trial()` is never called. The budget counter stays at 0 forever. With `num_trials=2`, PSO runs 20 particle evaluations instead of 1. + +- [ ] **Step 1: Call `increment_trial()` from `evaluate_point()`** + +In `optimizer.rs`, find `evaluate_point()` (or the function that calls `model.train_with_params()`). After each evaluation completes, increment the observer's trial counter. + +The challenge: `TrialBudgetObserver` is passed to `argmin::Executor` and the optimizer doesn't have direct access to it during evaluation. The fix: use an `Arc` shared between the observer and the cost function. + +```rust +// In ArgminOptimizer::optimize(): +let trial_counter = Arc::new(AtomicUsize::new(0)); + +// Pass to ObjectiveFunction: +let cost_fn = ObjectiveFunction { + model: adapter, + trial_counter: trial_counter.clone(), + max_trials: self.max_trials, +}; + +// Pass to TrialBudgetObserver: +let observer = TrialBudgetObserver::new(self.max_trials, trial_counter.clone()); + +// In ObjectiveFunction::cost(): +fn cost(&self, param: &Vec) -> Result { + let trial_num = self.trial_counter.fetch_add(1, Ordering::SeqCst); + if trial_num >= self.max_trials { + return Err(Error::msg("Trial budget exhausted")); + } + // ... train_with_params ... +} +``` + +- [ ] **Step 2: Update TrialBudgetObserver to use shared counter** + +```rust +pub struct TrialBudgetObserver { + max_trials: usize, + trial_counter: Arc, +} + +impl TrialBudgetObserver { + pub fn new(max_trials: usize, trial_counter: Arc) -> Self { + Self { max_trials, trial_counter } + } +} + +impl Observe for TrialBudgetObserver { + fn observe_iter(&mut self, _state: &I, _kv: &KV) -> Result<(), Error> { + let used = self.trial_counter.load(Ordering::SeqCst); + if used >= self.max_trials { + Err(Error::msg(format!("Trial budget exhausted: {used}/{}", self.max_trials))) + } else { + Ok(()) + } + } +} +``` + +- [ ] **Step 3: Build + test** + +Run: `SQLX_OFFLINE=true cargo test -p ml-hyperopt --lib` +Expected: optimizer tests pass, trial count is respected + +- [ ] **Step 4: Commit** + +```bash +git commit -m "fix: trial budget observer — shared AtomicUsize counter between cost fn and observer + +increment_trial() was never called — budget enforcement was broken. +Now the ObjectiveFunction::cost() increments the shared counter on each +evaluation, and TrialBudgetObserver checks it per PSO iteration. +With num_trials=2: exactly 2 evaluations (1 LHS + 1 PSO), not 21." +``` + +--- + +## Task 4: Make CVaR threshold dynamic + +**Files:** +- Modify: `crates/ml/src/hyperopt/adapters/dqn.rs` + +- [ ] **Step 1: Replace hardcoded `0.003` with dynamic computation** + +Find line ~3772: +```rust +let cvar_threshold = 0.003; +``` + +Replace with: +```rust +// CVaR threshold scales with bar frequency: 0.05 / sqrt(bars_per_day) +// 1-min (390): 0.003, 5-min (78): 0.006, daily (1): 0.05 +let bars_per_day = metrics.bars_per_day.unwrap_or(390.0); +let cvar_threshold = 0.05 / bars_per_day.sqrt(); +``` + +This requires adding `bars_per_day` to `DQNMetrics`. If that's too invasive, use the constant from `common::thresholds::time::BARS_PER_DAY`: + +```rust +let cvar_threshold = 0.05 / common::thresholds::time::BARS_PER_DAY.sqrt(); +``` + +- [ ] **Step 2: Build + test** + +Run: `SQLX_OFFLINE=true cargo test -p ml --lib -- objective` + +- [ ] **Step 3: Commit** + +```bash +git commit -m "fix: CVaR threshold scales with bars_per_day — was hardcoded at 0.003" +``` + +--- + +## Task 5: Extract trade threshold constant + fix stale test vectors + +**Files:** +- Modify: `crates/ml/src/hyperopt/adapters/dqn.rs` + +- [ ] **Step 1: Extract named constant for trade threshold** + +Find the two independent `10` values at lines ~2496 and ~3738. Replace both with: + +```rust +/// Minimum trades for non-degenerate trial (below this → full penalty). +const MIN_TRADES_DEGENERATE: usize = 10; +``` + +Use `MIN_TRADES_DEGENERATE` in both locations. + +- [ ] **Step 2: Fix stale test vectors** + +Search for `vec![0.0_f64; 39]`, `vec![0.0_f64; 41]`, `vec![0.0; 45]` in test code. Update to `vec![0.0_f64; 46]` (current 46D search space) or use `DQNParams::continuous_bounds().len()` dynamically: + +```rust +let dim = DQNParams::continuous_bounds().len(); +let mut vec = vec![0.0_f64; dim]; +``` + +- [ ] **Step 3: Build + test** + +Run: `SQLX_OFFLINE=true cargo test -p ml --lib -- hyperopt` + +- [ ] **Step 4: Commit** + +```bash +git commit -m "refactor: extract MIN_TRADES_DEGENERATE constant, fix stale 39D/41D test vectors" +``` + +--- + +## Task 6: Integration test + local hyperopt validation + +- [ ] **Step 1:** `SQLX_OFFLINE=true cargo test -p ml --lib` — 887+ pass +- [ ] **Step 2:** `SQLX_OFFLINE=true cargo test -p ml-dqn --lib` — 359 pass +- [ ] **Step 3:** `SQLX_OFFLINE=true cargo test -p ml-hyperopt --lib` — all pass +- [ ] **Step 4:** `SQLX_OFFLINE=true cargo check --workspace` — clean +- [ ] **Step 5:** Local hyperopt smoke test: + +```bash +rm -rf /tmp/.cubin_cache/ +FOXHUNT_TEST_DATA=/home/jgrusewski/Work/foxhunt/test_data/futures-baseline \ +SQLX_OFFLINE=true cargo test -p ml --lib --release -- test_local_hyperopt --ignored --nocapture +``` + +Expected: objective < 1000000 (not degenerate), trade count reasonable + +- [ ] **Step 6:** Push to main + +--- + +## Execution Order + +``` +Task 1 (backtest hold_time + enforcement) → Task 2 (document strategy) + → Task 3 (trial budget fix) → Task 4 (CVaR dynamic) + → Task 5 (constants + tests) → Task 6 (integration) +``` + +Tasks 1-2 fix the train/eval mismatch (root cause of 1e6 penalty). +Task 3 fixes the trial budget (root cause of 21 evals instead of 2). +Tasks 4-5 are cleanup. +Task 6 validates everything.