# 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 |
