docs: final fixes plan — 6 issues blocking H100 deployment
1. Backtest hold_time tracking + enforcement (train/eval mismatch root cause) 2. Document evaluator action masking strategy (Layer 2 in env_step) 3. Fix trial budget observer (shared AtomicUsize counter) 4. Dynamic CVaR threshold (0.05 / sqrt(bars_per_day)) 5. Extract MIN_TRADES_DEGENERATE constant + fix stale test vectors 6. Integration test + local hyperopt validation Root cause chain: broken trial budget → 21 evals → no hold in eval → NaN Sharpe → 1e6 penalty → "degenerate" result. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
jgrusewski
315
docs/superpowers/plans/2026-03-26-final-fixes-before-h100.md
Normal file
315
docs/superpowers/plans/2026-03-26-final-fixes-before-h100.md
Normal file
@@ -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<AtomicUsize>` 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<f64>) -> Result<f64, Error> {
|
||||
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<AtomicUsize>,
|
||||
}
|
||||
|
||||
impl TrialBudgetObserver {
|
||||
pub fn new(max_trials: usize, trial_counter: Arc<AtomicUsize>) -> Self {
|
||||
Self { max_trials, trial_counter }
|
||||
}
|
||||
}
|
||||
|
||||
impl<I> Observe<I> 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.
|
||||
Reference in New Issue
Block a user