From 75885bccafded624045493bf9c8e8bfb781ad3c8 Mon Sep 17 00:00:00 2001 From: jgrusewski Date: Sun, 22 Mar 2026 20:30:35 +0100 Subject: [PATCH] =?UTF-8?q?spec:=20address=20review=20=E2=80=94=20DSR=20fo?= =?UTF-8?q?rmula,=20div-by-zero=20guards,=20stride=20constant?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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) --- .../2026-03-22-gpu-composite-reward-design.md | 89 +++++++++++++------ 1 file changed, 61 insertions(+), 28 deletions(-) diff --git a/docs/superpowers/specs/2026-03-22-gpu-composite-reward-design.md b/docs/superpowers/specs/2026-03-22-gpu-composite-reward-design.md index faf592dfd..939fdc9ff 100644 --- a/docs/superpowers/specs/2026-03-22-gpu-composite-reward-design.md +++ b/docs/superpowers/specs/2026-03-22-gpu-composite-reward-design.md @@ -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` 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