docs: trade lifecycle fix spec — address review feedback
- Layer 1 now targets experience_action_select (GPU-fused training path) AND branching_action_select (backtest/fallback), not just the fallback - Action masking covers both greedy AND random exploration paths - Exposure index uses parameterized b0_size, not hardcoded 5 or 9 - Fixed end-of-episode variable name (total_bars, not timesteps_per_episode) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
173
docs/superpowers/specs/2026-03-25-trade-lifecycle-fix-design.md
Normal file
173
docs/superpowers/specs/2026-03-25-trade-lifecycle-fix-design.md
Normal file
@@ -0,0 +1,173 @@
|
||||
# Trade Lifecycle Fix — 4 Bugs, 3 Layers of Defense
|
||||
|
||||
## Problem
|
||||
|
||||
The DQN model does 620K trades over 800K bars (77% turnover) instead of multi-bar swing trading. Four bugs in the experience kernel combine to make per-bar trading the optimal policy:
|
||||
|
||||
1. **Hold enforcement is dead code** — the boolean condition `prev_sign != 0 && curr_sign == 0 && !exiting_trade` is always false because `exiting_trade` is defined as `prev_sign != 0 && curr_sign == 0`
|
||||
2. **Reversals bypass hold constraint** — the guard only checks `curr_sign == 0` (flat exits), so L100→S50 reversals fire on bar 2 with no minimum hold
|
||||
3. **Single-bar trades get full reward** — `segment_hold_time` is captured before `hold_time += 1.0`, so bar-2 exits with `hold_time=1.0` pass the `> 0.0f` guard
|
||||
4. **No action masking** — the agent can freely choose any action every bar; no mechanism prevents changing exposure during the hold period
|
||||
|
||||
## Design: 3 Layers of Defense
|
||||
|
||||
### Layer 1 — Action Masking (Primary, `epsilon_greedy_kernel.cu`)
|
||||
|
||||
Action masking must target **two kernels** — the GPU-fused training path and the CPU-side fallback:
|
||||
|
||||
**Primary target: `experience_action_select` in `experience_kernels.cu` (lines 352-428)**
|
||||
This is the kernel used during GPU-fused training — the path where the 620K trade problem manifests. The portfolio state (`ps[]`) is already available in this kernel. When `hold_time < min_hold_bars && hold_time > 0.0f`:
|
||||
|
||||
- **Greedy path:** Before computing argmax over exposure Q-values, mask all exposure indices except the current one by setting their Q-values to `-1e30f`
|
||||
- **Random exploration path:** When epsilon fires (line 380-382), restrict the random exposure selection to the current exposure index only (skip the random branch, use current exposure)
|
||||
- Leave order-type and urgency branches unmasked (agent can still choose order parameters while holding)
|
||||
|
||||
**Secondary target: `branching_action_select` in `epsilon_greedy_kernel.cu`**
|
||||
Used by the backtest evaluator and CPU-side `select_actions_batch`. Apply the same masking logic. Note: this kernel currently uses `b0_size` as a parameter (9 for production, may differ in tests), so the masking must use `b0_size` not a hardcoded 5 or 9.
|
||||
|
||||
Current exposure index derived from portfolio state: `exposure_idx = round((ps[0] / max_position + 1.0) * 0.5 * (b0_size - 1))`, mapping continuous position `[-max_pos, +max_pos]` to discrete `[0, b0_size-1]`.
|
||||
|
||||
**Kernel changes to `experience_action_select`:**
|
||||
- Add `int min_hold_bars` parameter
|
||||
- Add `float max_position` parameter (for position→exposure mapping)
|
||||
- Read `ps[base + 10]` for hold_time, `ps[base + 0]` for position (already available)
|
||||
- Mask Q-values before argmax AND restrict random exploration during hold
|
||||
|
||||
**Kernel changes to `branching_action_select`:**
|
||||
- Add `const float* portfolio_state` parameter (read-only, `[n_episodes * PORTFOLIO_DIM]`)
|
||||
- Add `int min_hold_bars` parameter
|
||||
- Add `float max_position` parameter
|
||||
- Read `ps[base + 10]` for hold_time, `ps[base + 0]` for position
|
||||
|
||||
### Layer 2 — Kernel Override (Safety Net, `experience_kernels.cu`)
|
||||
|
||||
Fix the dead code and extend the hold guard to cover both flat exits AND reversals.
|
||||
|
||||
**Bug 1 fix:** Remove the `!exiting_trade` term. Move the hold check BEFORE the position update logic:
|
||||
|
||||
```cuda
|
||||
// Hold enforcement: block ALL position changes during minimum hold period
|
||||
int wants_exit = (prev_sign != 0 && curr_sign == 0);
|
||||
int wants_reversal = (prev_sign != 0 && curr_sign != 0 && prev_sign != curr_sign);
|
||||
int hold_violation = (hold_time < (float)min_hold_bars && hold_time > 0.0f
|
||||
&& (wants_exit || wants_reversal));
|
||||
|
||||
if (hold_violation) {
|
||||
// Override: keep previous position, cancel the action
|
||||
position = ps[0];
|
||||
// Recompute signs from overridden position
|
||||
curr_sign = prev_sign;
|
||||
wants_exit = 0;
|
||||
wants_reversal = 0;
|
||||
}
|
||||
|
||||
// THEN compute exiting_trade and reversing_trade from the (possibly overridden) position
|
||||
int exiting_trade = wants_exit;
|
||||
int reversing_trade = wants_reversal;
|
||||
int segment_complete = exiting_trade || reversing_trade;
|
||||
```
|
||||
|
||||
**Bug 2 fix:** The `wants_exit || wants_reversal` in the hold_violation check covers both cases.
|
||||
|
||||
### Layer 3 — Reward Scaling (Gradient Signal, `experience_kernels.cu`)
|
||||
|
||||
For trades that complete with short hold times (possible when min_hold is bypassed by trailing stop or end-of-episode), scale the reward to give the optimizer a smooth gradient toward longer holds:
|
||||
|
||||
```cuda
|
||||
// After computing base reward (line ~927):
|
||||
if (segment_complete && segment_hold_time > 0.0f) {
|
||||
// ... existing reward computation ...
|
||||
|
||||
// Reward scaling: shorter trades get proportionally less reward
|
||||
float hold_scale = fminf(segment_hold_time / (float)min_hold_bars, 1.0f);
|
||||
hold_scale = fmaxf(hold_scale, 0.1f); // Floor at 10% to maintain gradient signal
|
||||
reward *= hold_scale;
|
||||
}
|
||||
```
|
||||
|
||||
A 1-bar trade gets 20% of a 5-bar trade's reward (with min_hold=5). A 3-bar trade gets 60%. Full reward at 5+ bars.
|
||||
|
||||
### Trailing Stop Interaction
|
||||
|
||||
The trailing stop (lines 779-792 in current kernel) activates when `hold_time > 2.0f`. With `min_hold_bars=5`, the trailing stop should respect the hold constraint:
|
||||
|
||||
- If trailing stop triggers AND `hold_time >= min_hold_bars`: exit allowed (trailing stop overrides hold for profit protection)
|
||||
- If trailing stop triggers AND `hold_time < min_hold_bars`: exit blocked (hold constraint takes priority over early profit-taking)
|
||||
|
||||
This means the trailing stop's `hold_time > 2.0f` check should be updated to `hold_time >= min_hold_bars`.
|
||||
|
||||
### Config Wiring
|
||||
|
||||
**`DQNHyperparameters` (`config.rs`):**
|
||||
```rust
|
||||
/// Minimum bars to hold a position before allowing exit or reversal.
|
||||
/// Prevents per-bar churning. Trailing stop can override after this period.
|
||||
/// Default: 5 (about 5 minutes for 1-min bars).
|
||||
pub min_hold_bars: usize,
|
||||
```
|
||||
|
||||
**`GpuExperienceCollectorConfig` (`gpu_experience_collector.rs`):**
|
||||
Add `min_hold_bars: i32` to the config, pass as kernel launch argument.
|
||||
|
||||
**`GpuActionSelector` (`gpu_action_selector.rs`):**
|
||||
Add `min_hold_bars: i32` and `portfolio_state_buf` to the action selection kernel launch.
|
||||
|
||||
**TOML (`config/training/*.toml`):**
|
||||
```toml
|
||||
[experience]
|
||||
min_hold_bars = 5 # minimum bars before exit/reversal allowed
|
||||
```
|
||||
|
||||
**Hyperopt (`hyperopt/adapters/dqn.rs`):**
|
||||
Add `min_hold_bars` to search space with bounds `[2, 20]`. Integer parameter (round from continuous).
|
||||
|
||||
### Portfolio State Flow for Action Masking
|
||||
|
||||
Currently the action selection kernel doesn't receive portfolio state. The flow:
|
||||
|
||||
1. Experience collector writes portfolio state to GPU buffer (`portfolio_buf`)
|
||||
2. Action selector needs to read `ps[0]` (position) and `ps[10]` (hold_time)
|
||||
3. Pass `portfolio_buf` as read-only parameter to the action selection kernel
|
||||
4. The buffer is already GPU-resident — zero-copy, just pass the pointer
|
||||
|
||||
### Expected Trade Count Impact
|
||||
|
||||
| Scenario | Trades | Bars/Trade |
|
||||
|----------|--------|------------|
|
||||
| Current (no hold) | 620K | 1.3 |
|
||||
| min_hold=3 | ~200K | 4.0 |
|
||||
| min_hold=5 | ~120K | 6.7 |
|
||||
| min_hold=10 | ~60K | 13.3 |
|
||||
| Realistic (learned) | ~30-80K | 10-27 |
|
||||
|
||||
The model will learn to trade at the natural timescale of ES futures moves (~5-15 minute swings).
|
||||
|
||||
### Edge Cases
|
||||
|
||||
**End of episode:** When `bar_idx >= total_bars - 1`, the position is forcibly closed. This should bypass the hold constraint (episode boundary is not a trading decision).
|
||||
|
||||
**Entering a position:** `hold_time == 0.0` means no position held. The masking should NOT fire (the agent must be free to enter). The guard `hold_time > 0.0f` ensures this.
|
||||
|
||||
**Same-direction resizing:** L100→L50 has `prev_sign == curr_sign`, so neither `wants_exit` nor `wants_reversal` is true. The hold constraint does NOT block same-direction resizing. This is intentional — the agent can manage position size within a trade.
|
||||
|
||||
**Flat→Flat:** `prev_sign == 0 && curr_sign == 0`. Hold time is 0 (reset on exit). No constraint fires. Correct.
|
||||
|
||||
## Files Modified
|
||||
|
||||
| File | Change |
|
||||
|------|--------|
|
||||
| `crates/ml/src/cuda_pipeline/experience_kernels.cu` | Fix hold guard, extend to reversals, add min_hold_bars param, reward scaling, trailing stop update, action masking in experience_action_select (primary training path) |
|
||||
| `crates/ml/src/cuda_pipeline/epsilon_greedy_kernel.cu` | Action masking in branching_action_select (backtest/fallback path), accept portfolio state + min_hold_bars |
|
||||
| `crates/ml/src/cuda_pipeline/gpu_experience_collector.rs` | Pass min_hold_bars to kernel launch |
|
||||
| `crates/ml/src/cuda_pipeline/gpu_action_selector.rs` | Pass portfolio state + min_hold_bars to action kernel |
|
||||
| `crates/ml/src/trainers/dqn/config.rs` | Add min_hold_bars to DQNHyperparameters |
|
||||
| `crates/ml/src/training_profile.rs` | Add min_hold_bars to TOML experience section |
|
||||
| `crates/ml/src/hyperopt/adapters/dqn.rs` | Add min_hold_bars to search space [2, 20] |
|
||||
| `config/training/dqn-smoketest.toml` | Add min_hold_bars = 3 |
|
||||
| `config/training/dqn-production.toml` | Add min_hold_bars = 5 |
|
||||
|
||||
## Risks
|
||||
|
||||
- **Reduced exploration early in training:** Masking limits the agent's action space during holds. The noisy nets and entropy regularization still drive exploration within the unmasked actions (order type, urgency). The hold constraint makes exploration slower but higher quality.
|
||||
- **Trailing stop timing:** Changing the trailing stop threshold from `> 2.0f` to `>= min_hold_bars` delays profit protection. With `min_hold_bars=5` on 1-minute bars, the model holds for at least 5 minutes before the trailing stop activates. For ES futures with typical 10-15 minute swings, this is acceptable.
|
||||
- **Hyperopt interaction:** The search space for `min_hold_bars` interacts with `gpu_timesteps_per_episode` (total bars per episode). If `min_hold_bars > timesteps/2`, the agent can barely trade. The search bound [2, 20] with typical `timesteps=100-500` keeps this safe.
|
||||
Reference in New Issue
Block a user