spec: address review — DSR formula, div-by-zero guards, stride constant

- DSR uses pre-update A/B values (Moody & Saffell correct)
- peak_equity/prev_equity init to initial_capital (not zero)
- Division-by-zero guards on drawdown and return calculation
- PORTFOLIO_STRIDE=12 constant replaces hardcoded 3 (7 locations listed)
- Remove use_dsr flag (w_dsr>0 implies enabled)
- RewardSection struct needed in training_profile.rs
- All TOML profiles use same [reward] section

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
jgrusewski
2026-03-22 20:30:35 +01:00
parent 530edd76b9
commit 75885bccaf

View File

@@ -56,20 +56,28 @@ reward = eff_w_dsr * dsr_t // Sharpe contribution
Per-step incremental Sharpe contribution (Moody & Saffell 2001):
```
R_t = pnl_t / equity_{t-1} // per-step return (not raw PnL)
A_t = A_{t-1} + eta * (R_t - A_{t-1}) // EMA of returns
B_t = B_{t-1} + eta * (R_t^2 - B_{t-1}) // EMA of squared returns
// Return (guard division by zero on prev_equity)
equity_denom = max(prev_equity, 1.0)
R_t = pnl_t / equity_denom
denominator = (B_t - A_t^2)^(3/2) // volatility^3
if |denominator| > epsilon:
dsr_t = (B_{t-1} * (R_t - A_{t-1}) - 0.5 * A_{t-1} * (R_t^2 - B_{t-1})) / denominator
else:
dsr_t = 0.0 // degenerate (no variance yet)
// Compute deltas BEFORE updating EMA (pre-update values for numerator+denominator)
delta_A = R_t - A_{t-1}
delta_B = R_t^2 - B_{t-1}
variance = B_{t-1} - A_{t-1}^2
denominator = max(|variance|, epsilon)^(3/2)
// DSR from pre-update state (Moody & Saffell correct formulation)
dsr_t = (B_{t-1} * delta_A - 0.5 * A_{t-1} * delta_B) / denominator
// NOW update EMA
A_t = A_{t-1} + eta * delta_A
B_t = B_{t-1} + eta * delta_B
```
**State**: 2 floats per episode (A_t, B_t)
**Cost**: ~10 FLOPs per step
**Why**: Dense reward that directly optimizes annualized Sharpe ratio. Each step gets immediate feedback on whether this action improved or degraded the portfolio's risk-adjusted return.
**Edge cases**: `prev_equity <= 0` → clamp denominator to 1.0. `variance ≈ 0` (no return history) → `dsr_t ≈ 0` (epsilon guard). Degenerate case returns 0.0 (not the CPU's `(r*100).clamp(-5,5)` — GPU is the canonical implementation).
### Component 2: Normalized PnL
@@ -89,11 +97,14 @@ normalized_pnl = (raw_pnl - pnl_ema) / pnl_std // z-scored PnL
```
equity = cash + position * close
peak_equity = max(peak_equity, equity)
drawdown = (peak_equity - equity) / peak_equity // fractional drawdown [0, 1]
drawdown_penalty_t = max(0, drawdown - dd_threshold) // only penalize beyond threshold
drawdown = peak_equity > 0.0
? (peak_equity - equity) / peak_equity // fractional drawdown [0, 1]
: 0.0 // guard: no equity = no drawdown
drawdown_penalty_t = max(0, drawdown - dd_threshold) // only penalize beyond threshold
```
**State**: 1 float per episode (peak_equity)
**Init**: `peak_equity = initial_capital` (NOT zero — zero causes div-by-zero on first step)
**Parameter**: `dd_threshold` (default 0.02 = 2% drawdown tolerance before penalty)
**Why**: The model must learn to cut losing positions. Without drawdown awareness, it holds losers hoping for recovery — exactly the behavior that produces 100% max drawdown.
@@ -169,27 +180,45 @@ No change from current implementation. Already correct.
## GPU State Layout
Introduce `PORTFOLIO_STRIDE = 12` constant (replaces all hardcoded `3`).
Extend the existing `portfolio_states` buffer from 3 floats to 12 floats per episode:
```
Index Current New
[0] position position (unchanged)
[1] cash cash (unchanged)
[2] portfolio_val portfolio_val (unchanged)
[3] dsr_A (EMA of returns)
[4] dsr_B (EMA of squared returns)
[5] pnl_ema (running mean PnL)
[6] pnl_var (running variance PnL)
[7] peak_equity (high-water mark)
[8] — flat_counter (consecutive flat steps)
[9] prev_equity (equity at previous step, for return calc)
[10] hold_time (consecutive steps with position)
[11] — realized_pnl (cumulative realized PnL for win-streak tracking)
Index Field Init Value Description
[0] position 0.0 current position size
[1] cash initial_capital available cash
[2] portfolio_val initial_capital mark-to-market equity
[3] dsr_A 0.0 EMA of returns
[4] dsr_B 0.0 EMA of squared returns
[5] pnl_ema 0.0 running mean PnL
[6] pnl_var 0.0 running variance PnL
[7] peak_equity initial_capital high-water mark (NOT zero — causes div/0)
[8] flat_counter 0.0 consecutive flat steps (float for GPU)
[9] prev_equity initial_capital equity at previous step (NOT zero — causes div/0)
[10] hold_time 0.0 consecutive steps with position
[11] realized_pnl 0.0 cumulative realized PnL
```
**Memory impact**: 9 extra floats × 2048 episodes × 4 bytes = 73 KB. Negligible.
**PORTFOLIO_DIM constant**: Update from 3 to 12 in `experience_kernels.cu` and all callers.
### Code locations requiring stride update (3 → PORTFOLIO_STRIDE)
All 7 hardcoded stride-3 references that MUST change:
| File | Line | Current | Change to |
|------|------|---------|-----------|
| `experience_kernels.cu` | 169 | `portfolio_states + i * 3` | `i * PORTFOLIO_STRIDE` |
| `experience_kernels.cu` | 420 | `portfolio_states + i * 3` | `i * PORTFOLIO_STRIDE` |
| `gpu_experience_collector.rs` | 575 | `alloc_episodes * 3` | `alloc_episodes * PORTFOLIO_STRIDE` |
| `gpu_experience_collector.rs` | 579 | `vec![0.0; alloc_episodes * 3]` | `alloc_episodes * PORTFOLIO_STRIDE` |
| `gpu_experience_collector.rs` | 581 | `i * 3` | `i * PORTFOLIO_STRIDE` |
| `gpu_experience_collector.rs` | 1166 | `alloc_episodes * 3` | `alloc_episodes * PORTFOLIO_STRIDE` |
| `gpu_experience_collector.rs` | 1168 | `i * 3` | `i * PORTFOLIO_STRIDE` |
The existing `PORTFOLIO_STATE_SIZE = 8` at `gpu_experience_collector.rs:51` is for the PPO/legacy path (NOT used by DQN experience collection). Do not change it — introduce a new `const PORTFOLIO_STRIDE: usize = 12` for the DQN path.
Note: `experience_kernels.cu` contains TWO different portfolio concepts in the same file — `experience_env_step` uses stride-3 (DQN, changing to 12), `portfolio_sim_kernel` uses stride-8 (legacy, unchanged).
## Hyperopt Search Space
@@ -248,7 +277,7 @@ All existing code that reads `portfolio_states[0..3]` must update to stride 12.
## TOML Configuration
Add to `config/training/dqn-production.toml`:
Add to ALL training profiles (`dqn-production.toml`, `dqn-smoketest.toml`, `dqn-hyperopt.toml`):
```toml
[reward]
w_dsr = 1.0
@@ -256,9 +285,12 @@ w_pnl = 0.3
w_dd = 1.0
w_idle = 0.01
dd_threshold = 0.02
eta = 0.01
loss_aversion = 1.5
time_decay_rate = 0.0005
```
Requires new `RewardSection` struct in `training_profile.rs` with `Option<f64>` fields, and `apply_to()` mapping to `DQNHyperparameters` reward fields.
Add to `config/training/dqn-hyperopt.toml` `[search_space]`:
```toml
w_dsr = [0.1, 2.0]
@@ -270,10 +302,11 @@ dd_threshold = [0.01, 0.10]
## What Gets Removed
1. **`hold_reward` / `hold_penalty`** — replaced by `idle_penalty` (inverted semantics, proper scaling)
1. **`hold_reward` / `hold_penalty` / `hold_penalty_weight`** — replaced by `idle_penalty` (inverted semantics). The kernel parameter `hold_rw` at `gpu_experience_collector.rs:1002-1020` is removed from the launch args.
2. **CPU `RewardNormalizer`** in `reward.rs` — normalization now in GPU kernel (pnl_ema/pnl_var)
3. **CPU DSR code** in `reward.rs` — DSR now in GPU kernel (dsr_A/dsr_B)
4. **`hold_penalty_weight`** from hyperopt search space — replaced by `w_idle`
4. **`use_dsr` bool flag** in `ExperienceCollectorConfig` — redundant when `w_dsr > 0` implies DSR enabled. Remove dual-control confusion.
5. **`hold_penalty_weight`** from hyperopt search space — replaced by `w_idle`
## What Stays