docs: add DQN hyperopt overhaul implementation plan

7 tasks covering: 5-action compatibility in eval/backtest paths, C2 triple exploration stacking fix (remove count bonus from Q-values, narrow epsilon floor, reduce search space 30D→29D), PPO isolation verification. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-03-06 14:32:37 +01:00
parent d1601b3720
commit e3c3cf0fa3
1 changed files with 428 additions and 0 deletions
--- a/docs/plans/2026-03-06-dqn-hyperopt-overhaul-implementation.md
+++ b/docs/plans/2026-03-06-dqn-hyperopt-overhaul-implementation.md
@@ -0,0 +1,428 @@
+# DQN Hyperopt Overhaul Implementation Plan
+
+> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task.
+
+**Goal:** Fix remaining hyperopt-to-eval mismatches so hyperopt configs produce consistent out-of-sample results.
+
+**Architecture:** Three workstreams: (1) fix 5-action compatibility bugs in eval/backtest paths introduced by the action-diversity work, (2) remove count bonus from Q-value computation to eliminate triple exploration stacking, (3) simplify epsilon to noisy nets only during training.
+
+**Tech Stack:** Rust, Candle ML, DQN, factored action space (5 exposure levels)
+
+**Pre-Implementation Audit:** B1 (softmax eval), B2 (NormStats), B3 (portfolio sync), C1 (extrinsic-only replay), C3 (neutral hold), C4 (Sharpe stopping) are already implemented in prior commits. This plan covers the remaining gaps.
+
+---
+
+## Task 1: Fix `FactoredAction::from_index()` in Hyperopt Backtest
+
+**Root Cause:** `hyperopt/adapters/dqn.rs:2940` calls `FactoredAction::from_index(action_idx)` with DQN output indices 0-4. Since the 45-action mapping has `exposure_idx = idx / 9`, indices 0-4 ALL map to `Short100` with different order/urgency combos. The hyperopt backtest silently runs every trial all-short.
+
+**Files:**
+- Modify: `crates/ml/src/hyperopt/adapters/dqn.rs`
+
+**Step 1: Write the failing test**
+
+Add test in the adapter's test module that verifies DQN exposure indices 0-4 produce distinct exposure levels in the backtest path:
+
+```rust
+#[test]
+fn test_exposure_index_maps_to_correct_action() {
+    use crate::dqn::action_space::ExposureLevel;
+    use crate::dqn::order_router::OrderRouter;
+
+    // DQN outputs 0-4 (exposure indices). Each must map to a distinct exposure.
+    let exposures: Vec<ExposureLevel> = (0..5)
+        .map(|idx| ExposureLevel::from_index(idx).unwrap())
+        .collect();
+
+    assert_eq!(exposures[0], ExposureLevel::Short100);
+    assert_eq!(exposures[1], ExposureLevel::Short50);
+    assert_eq!(exposures[2], ExposureLevel::Flat);
+    assert_eq!(exposures[3], ExposureLevel::Long50);
+    assert_eq!(exposures[4], ExposureLevel::Long100);
+
+    // All must produce valid FactoredActions
+    for idx in 0..5 {
+        let exposure = ExposureLevel::from_index(idx).unwrap();
+        let action = OrderRouter::route_default(exposure);
+        assert_eq!(action.exposure, exposure);
+    }
+}
+```
+
+**Step 2: Run test to verify it passes (sanity check on ExposureLevel)**
+
+Run: `SQLX_OFFLINE=true cargo test -p ml --lib test_exposure_index_maps_to_correct_action -- --exact`
+Expected: PASS (ExposureLevel::from_index already works correctly)
+
+**Step 3: Fix the backtest action conversion**
+
+In `crates/ml/src/hyperopt/adapters/dqn.rs`, line ~2940, replace:
+```rust
+let factored = crate::dqn::FactoredAction::from_index(action_idx)?;
+```
+with:
+```rust
+let exposure = crate::dqn::action_space::ExposureLevel::from_index(action_idx)?;
+let factored = crate::dqn::OrderRouter::route_default(exposure);
+```
+
+**Step 4: Fix unique_actions logging**
+
+Line ~2985: Change `unique_actions={}/{}, 45` to `unique_actions={}/{}, 5`
+
+**Step 5: Run tests**
+
+Run: `SQLX_OFFLINE=true cargo test -p ml --lib -- hyperopt::adapters::dqn`
+Expected: All hyperopt DQN adapter tests pass
+
+**Step 6: Commit**
+
+```bash
+git add crates/ml/src/hyperopt/adapters/dqn.rs
+git commit -m "fix(ml): hyperopt backtest uses ExposureLevel, not FactoredAction::from_index
+
+FactoredAction::from_index(0-4) maps all to Short100. ExposureLevel::from_index(0-4)
+correctly maps to 5 distinct exposure levels."
+```
+
+---
+
+## Task 2: Fix `FactoredAction::from_index()` in evaluate_baseline
+
+**Root Cause:** Same bug as Task 1 but in the standalone evaluation binary. `evaluate_baseline.rs:466` calls `FactoredAction::from_index(action_idx)` with DQN indices 0-4, producing all-Short100 evaluations.
+
+**Files:**
+- Modify: `crates/ml/examples/evaluate_baseline.rs`
+
+**Step 1: Fix the CLI default**
+
+Line ~97: Change `default_value_t = 45` to `default_value_t = 5` for `num_actions`. Update the doc comment to reflect DQN uses 5 exposure levels.
+
+Note: PPO callers must now explicitly pass `--num-actions 45`. Add a note to the `--help` text.
+
+**Step 2: Fix simulate_chunk_trades action conversion**
+
+Line ~466: Replace:
+```rust
+let action = match FactoredAction::from_index(action_idx) {
+    Ok(fa) => fa,
+    Err(e) => {
+        warn!("  [{}] from_index({}) error at bar {}: {}", model_name, action_idx, bar_idx, e);
+        FactoredAction::new(ExposureLevel::Flat, ActionOrderType::Market, Urgency::Normal)
+    }
+};
+```
+with:
+```rust
+let action = match ExposureLevel::from_index(action_idx) {
+    Ok(exposure) => ml::dqn::OrderRouter::route_default(exposure),
+    Err(e) => {
+        warn!("  [{}] exposure_from_index({}) error at bar {}: {}", model_name, action_idx, bar_idx, e);
+        FactoredAction::new(ExposureLevel::Flat, ActionOrderType::Market, Urgency::Normal)
+    }
+};
+```
+
+**IMPORTANT**: Check if `simulate_chunk_trades` is also called from the PPO eval path. If PPO calls it with indices 0-44, this function needs a `model_type` parameter to dispatch correctly. Read the PPO eval section (~lines 774-828) to confirm.
+
+**Step 3: Fix DQN config in eval**
+
+Line ~580: Verify `DQNConfig { num_actions: args.num_actions }` — with the default changed to 5, this should now be correct. But the PPO path must NOT use this config.
+
+**Step 4: Build check**
+
+Run: `SQLX_OFFLINE=true cargo check --workspace`
+Expected: 0 errors, 0 warnings
+
+**Step 5: Commit**
+
+```bash
+git add crates/ml/examples/evaluate_baseline.rs
+git commit -m "fix(ml): evaluate_baseline uses ExposureLevel for DQN 5-action space
+
+DQN default num_actions=5. FactoredAction::from_index() replaced with
+ExposureLevel::from_index() + OrderRouter. PPO must pass --num-actions 45."
+```
+
+---
+
+## Task 3: Remove Count Bonus from Q-Value Computation (C2a)
+
+**Root Cause:** Count bonus (`broadcast_add` of UCB bonuses to Q-values before argmax) creates a second exploration mechanism on top of noisy nets + epsilon floor. This triple stacking produces overly exploratory training behavior, inflating trade counts in hyperopt trials (58K trades in 3 months).
+
+**Files:**
+- Modify: `crates/ml/src/trainers/dqn/trainer.rs`
+
+**Step 1: Remove count bonus from `select_actions_batch()`**
+
+Lines ~3292-3307: Replace the count bonus block:
+```rust
+// Apply count bonus (UCB exploration) to Q-values before argmax.
+// Previously only in DQN::select_action() (single-sample path),
+// making the batch training path have no UCB exploration.
+let use_cb = agent.use_count_bonus();
+let n_actions = agent.num_actions();
+let batch_q_values = if use_cb {
+    let bonuses = agent.get_count_bonuses();
+    let bonus_tensor = Tensor::from_vec(bonuses, (1, n_actions), batch_q_values.device())
+        .map_err(|e| anyhow::anyhow!("Failed to create bonus tensor: {}", e))?
+        .to_dtype(batch_q_values.dtype())
+        .map_err(|e| anyhow::anyhow!("Failed to cast bonus tensor: {}", e))?;
+    batch_q_values.broadcast_add(&bonus_tensor)
+        .map_err(|e| anyhow::anyhow!("Failed to add count bonus: {}", e))?
+} else {
+    batch_q_values
+};
+```
+with:
+```rust
+// C2 FIX: Count bonus removed from Q-value computation.
+// Noisy nets are the sole exploration mechanism during training.
+// Count bonus kept for diversity metrics only (record_action tracking).
+```
+
+**Step 2: Remove count bonus from `select_actions_batch_gpu()`**
+
+Lines ~3367-3381: Same removal as Step 1. Replace the identical count bonus block with the same comment.
+
+**Step 3: Keep `record_action()` calls intact**
+
+Verify that `agent.record_action()` / `count_bonus.record_action()` calls remain in place after action selection. These track diversity metrics but no longer affect Q-values. Do NOT remove:
+- `trainer.rs` calls to `reset_count_bonus()` at epoch boundary (line ~2717)
+- Any `record_action()` calls post action selection
+
+**Step 4: Run tests**
+
+Run: `SQLX_OFFLINE=true cargo test -p ml --lib`
+Expected: All 2728+ tests pass
+
+**Step 5: Commit**
+
+```bash
+git add crates/ml/src/trainers/dqn/trainer.rs
+git commit -m "fix(ml): remove count bonus from Q-value computation (C2)
+
+Noisy nets are sole exploration mechanism during training.
+Count bonus UCB removed from argmax input in both batch paths.
+Count bonus module kept for diversity metrics tracking only."
+```
+
+---
+
+## Task 4: Remove Epsilon Floor from Training (C2b)
+
+**Root Cause:** `noisy_epsilon_floor` (default 0.05 = 5%) adds random epsilon-greedy exploration on top of noisy nets. With noisy nets providing learned, state-dependent exploration and only 5 actions, the epsilon floor is unnecessary noise.
+
+**Files:**
+- Modify: `crates/ml/src/hyperopt/adapters/dqn.rs`
+
+**Step 1: Set noisy_epsilon_floor default to 0.0**
+
+Line ~465: Change:
+```rust
+noisy_epsilon_floor: 0.05, // Minimum random exploration with noisy nets
+```
+to:
+```rust
+noisy_epsilon_floor: 0.0, // C2: Noisy nets provide exploration; no epsilon floor during training
+```
+
+**Step 2: Narrow the search space bounds**
+
+Line ~518: Change:
+```rust
+(0.02, 0.10),       // 23: noisy_epsilon_floor (linear)
+```
+to:
+```rust
+(0.0, 0.05),        // 23: noisy_epsilon_floor (linear, 0.0=noisy-nets-only)
+```
+
+This allows TPE to discover if any epsilon floor helps, but biases toward 0. The lower bound 0.0 lets TPE select pure noisy-net exploration.
+
+**Step 3: Update clamp bounds**
+
+Line ~583: Change:
+```rust
+let noisy_epsilon_floor = x[23].clamp(0.02, 0.10);
+```
+to:
+```rust
+let noisy_epsilon_floor = x[23].clamp(0.0, 0.05);
+```
+
+**Step 4: Update test assertions**
+
+Update test bounds assertions (~line 3620):
+```rust
+assert_eq!(bounds[23], (0.0, 0.05));     // noisy_epsilon_floor
+```
+
+Update test default value assertions and from_continuous test vectors for index 23.
+
+**Step 5: Run tests**
+
+Run: `SQLX_OFFLINE=true cargo test -p ml --lib -- hyperopt::adapters::dqn`
+Expected: All pass after assertion updates
+
+**Step 6: Commit**
+
+```bash
+git add crates/ml/src/hyperopt/adapters/dqn.rs
+git commit -m "fix(ml): narrow noisy_epsilon_floor to [0.0, 0.05] (C2)
+
+Default 0.0 = noisy nets only. TPE can still discover small epsilon
+if beneficial, but no longer forced to 5-10% random exploration."
+```
+
+---
+
+## Task 5: Remove count_bonus_coefficient from Hyperopt Search Space (C2c)
+
+**Root Cause:** Count bonus no longer modifies Q-values (Task 3). The coefficient is still in the 30D search space, wasting a dimension that TPE must explore. Removing it reduces 30D → 29D.
+
+**Files:**
+- Modify: `crates/ml/src/hyperopt/adapters/dqn.rs`
+
+**Step 1: Analyze the impact**
+
+Before changing, verify:
+1. `count_bonus_coefficient` is at index 29 (last dimension)
+2. Removing it shifts NO other indices (it's last)
+3. All `from_continuous()` / `to_continuous()` / `continuous_bounds()` / `param_names()` must be updated
+4. Test vectors in test module must be updated
+
+**Step 2: Remove dimension 29**
+
+In `continuous_bounds()`: Remove `(0.05, 1.0), // 29: count_bonus_coefficient` entry.
+
+In `from_continuous()`:
+- Change length check from 30 to 29
+- Remove `let count_bonus_coefficient = x[29].clamp(0.05, 1.0);`
+- Hardcode: `count_bonus_coefficient: 0.1` (fixed default, only used for diversity metrics)
+
+In `to_continuous()`:
+- Remove `self.count_bonus_coefficient, // 29`
+- Output Vec shrinks from 30 to 29
+
+In `param_names()`:
+- Remove `"count_bonus_coefficient", // 29`
+
+**Step 3: Update all comments**
+
+Change "30D" → "29D" in all comments throughout the file.
+
+**Step 4: Update all tests**
+
+- Bounds test: `assert_eq!(bounds.len(), 29)`
+- Names test: `assert_eq!(names.len(), 29)`
+- from_continuous vectors: reduce from 30 to 29 elements
+- Roundtrip test: verify count_bonus_coefficient is fixed at 0.1
+
+**Step 5: Run tests**
+
+Run: `SQLX_OFFLINE=true cargo test -p ml --lib -- hyperopt::adapters::dqn`
+Expected: All pass after updates
+
+**Step 6: Commit**
+
+```bash
+git add crates/ml/src/hyperopt/adapters/dqn.rs
+git commit -m "fix(ml): remove count_bonus_coefficient from search space (29D)
+
+Count bonus no longer modifies Q-values (C2). Coefficient fixed at 0.1
+for diversity metrics only. Search space reduced from 30D to 29D."
+```
+
+---
+
+## Task 6: Verify PPO Isolation
+
+**Root Cause:** PPO must remain at 45 actions. Verify no PPO paths were broken by the changes.
+
+**Files:**
+- Read: `crates/ml/examples/evaluate_baseline.rs` (PPO eval section)
+- Read: `crates/ml/src/cuda_pipeline/ppo_experience_kernel.cu`
+
+**Step 1: Verify PPO eval path uses 45 actions**
+
+Read evaluate_baseline.rs lines ~774-828 (PPO section). Confirm:
+- PPO config uses `num_actions: 45` (or reads from checkpoint)
+- PPO action indices 0-44 are correctly handled
+- If PPO shares `simulate_chunk_trades()`, confirm it receives 0-44 indices and the function handles both DQN (0-4) and PPO (0-44) correctly
+
+If `simulate_chunk_trades()` is shared: add a parameter to dispatch between `ExposureLevel::from_index()` (DQN) and `FactoredAction::from_index()` (PPO).
+
+**Step 2: Verify CUDA PPO kernel**
+
+Confirm `ppo_experience_kernel.cu` still uses `PPO_NUM_ACTIONS=45` and `ppo_action_to_exposure()`.
+
+**Step 3: Run PPO-specific tests**
+
+Run: `SQLX_OFFLINE=true cargo test -p ml --lib -- ppo`
+Expected: All PPO tests pass
+
+**Step 4: Run full workspace build**
+
+Run: `SQLX_OFFLINE=true cargo check --workspace`
+Expected: 0 errors, 0 warnings
+
+**Step 5: Commit (if changes needed)**
+
+Only commit if PPO isolation required code changes.
+
+---
+
+## Task 7: Full Test Suite + Clippy Verification
+
+**Files:** None (verification only)
+
+**Step 1: Full test suite**
+
+Run: `SQLX_OFFLINE=true cargo test -p ml --lib`
+Expected: 2728+ passed, 0 failed
+
+**Step 2: Clippy**
+
+Run: `SQLX_OFFLINE=true cargo clippy --workspace -- -D warnings`
+Expected: 0 errors, 0 warnings
+
+**Step 3: Final commit**
+
+```bash
+git add -A
+git commit -m "chore(ml): DQN hyperopt overhaul complete
+
+B1-B3 (eval alignment): verified already implemented.
+C1 (extrinsic-only replay): verified already implemented.
+C2 (single exploration): count bonus removed from Q-values,
+    epsilon floor narrowed to [0.0, 0.05], search space 29D.
+C3 (neutral hold): verified already implemented.
+C4 (Sharpe stopping): verified already implemented.
+5-action compatibility: fixed in hyperopt backtest + evaluate_baseline."
+```
+
+---
+
+## Verification Checklist
+
+After all tasks:
+
+| Fix | Verification |
+|-----|-------------|
+| 5-action eval | `evaluate_baseline --model dqn` produces 5 distinct exposure actions |
+| 5-action hyperopt backtest | Hyperopt trial backtest shows unique_actions across all 5 exposures |
+| C2 count bonus | Q-values in batch paths have no UCB bonus added |
+| C2 epsilon floor | `noisy_epsilon_floor` defaults to 0.0, range [0.0, 0.05] |
+| C2 search space | 29D (count_bonus_coefficient removed) |
+| PPO isolation | PPO tests pass, PPO still uses 45 actions |
+| No regressions | 2728+ tests, 0 clippy warnings |
+
+## Risk Assessment
+
+| Risk | Mitigation |
+|------|-----------|
+| Removing count bonus reduces diversity | 5-action space is small enough for noisy nets; diversity penalty in objective still catches collapse |
+| Removing epsilon floor causes exploitation collapse | Noisy nets provide state-dependent exploration; TPE can still select small epsilon if needed (range [0.0, 0.05]) |
+| evaluate_baseline shared between DQN/PPO | Task 6 verifies PPO isolation; may need model_type dispatch in simulate_chunk_trades |
+| 29D search space invalidates prior hyperopt results | Expected — prior results used degenerate 45-action space anyway |