From 84f8543a5f9fd10cdb103262f9a327c63ea4b592 Mon Sep 17 00:00:00 2001 From: jgrusewski Date: Wed, 25 Mar 2026 23:13:08 +0100 Subject: [PATCH] =?UTF-8?q?fix:=20hold=20enforcement=20in=20experience=5Fe?= =?UTF-8?q?nv=5Fstep=20=E2=80=94=20blocks=20exits=20AND=20reversals=20duri?= =?UTF-8?q?ng=20min=5Fhold?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Layer 2 (safety net): unified hold guard replaces dead code boolean. Old guard (prev_sign!=0 && curr_sign==0 && !exiting_trade) was always false. New guard covers both flat exits and reversals. Layer 3 (gradient): reward scaled by hold_time/min_hold_bars (floor 10%). Trailing stop respects min_hold_bars (was hardcoded > 2.0f). Co-Authored-By: Claude Sonnet 4.6 --- .../src/cuda_pipeline/experience_kernels.cu | 69 +++++++++++++------ .../cuda_pipeline/gpu_experience_collector.rs | 5 ++ .../src/trainers/dqn/trainer/training_loop.rs | 1 + 3 files changed, 54 insertions(+), 21 deletions(-) diff --git a/crates/ml/src/cuda_pipeline/experience_kernels.cu b/crates/ml/src/cuda_pipeline/experience_kernels.cu index 3badeaa65..fa46d0d62 100644 --- a/crates/ml/src/cuda_pipeline/experience_kernels.cu +++ b/crates/ml/src/cuda_pipeline/experience_kernels.cu @@ -486,6 +486,7 @@ extern "C" __global__ void experience_action_select( * @param cvar_scales [N] or NULL CVaR position scaling * @param q_gaps [N] or NULL Q-gap conviction scaling * @param raw_returns_out [N, L] or NULL true per-bar portfolio return (unshaped) + * @param min_hold_bars minimum bars to hold before exiting or reversing */ extern "C" __global__ void experience_env_step( const float* __restrict__ targets, @@ -513,7 +514,8 @@ extern "C" __global__ void experience_env_step( int current_t, const float* __restrict__ cvar_scales, /* [N] or NULL — CVaR position scaling */ const float* __restrict__ q_gaps, /* [N] or NULL — Q-gap conviction scaling */ - float* raw_returns_out /* [N, L] output: true per-bar portfolio return (unshapen) */ + float* raw_returns_out, /* [N, L] output: true per-bar portfolio return (unshapen) */ + int min_hold_bars /* minimum bars to hold before exiting or reversing */ ) { int i = blockIdx.x * blockDim.x + threadIdx.x; if (i >= N) return; @@ -713,12 +715,9 @@ extern "C" __global__ void experience_env_step( int curr_sign = (position > 0.001f) ? 1 : ((position < -0.001f) ? -1 : 0); int entering_trade = (prev_sign == 0 && curr_sign != 0); - int exiting_trade = (prev_sign != 0 && curr_sign == 0); + int exiting_trade = 0; /* preliminary — trailing stop may set to 1, hold guard recomputes */ int reversing_trade = (prev_sign != 0 && curr_sign != 0 && prev_sign != curr_sign); - /* A segment completes on exit OR reversal */ - int segment_complete = exiting_trade || reversing_trade; - /* On reversal: close old segment, open new one. * reversal_return is saved for the sparse reward block below * (trade_start_pnl is overwritten here, so recomputing would yield 0). */ @@ -776,8 +775,8 @@ extern "C" __global__ void experience_env_step( float trail_distance = 0.005f * vol_scale * trend_scale; /* 0.5% base, widens in vol/trend */ - /* Trailing stop: only when profitable and held > 2 bars */ - if (prev_sign != 0 && hold_time > 2.0f && peak_equity > 1.0f) { + /* Trailing stop: only when profitable and held >= min_hold_bars */ + if (prev_sign != 0 && hold_time >= (float)min_hold_bars && peak_equity > 1.0f) { float peak_return = (peak_equity - prev_equity) / (prev_equity > 1.0f ? prev_equity : 1.0f); if (peak_return > trail_distance) { float trail_floor = peak_return - trail_distance; @@ -792,31 +791,50 @@ extern "C" __global__ void experience_env_step( } } - /* Early-exit override: model tries to exit to FLAT before 5 bars — force hold. - * Only blocks flat exits, NOT reversals — reversals always allowed. - * CRITICAL: fix the stored action to match what actually executed. - * Without this, the replay buffer stores (state, action=Flat, reward_from_Hold) - * which corrupts Q-values for Flat — classic action aliasing bug. - * TODO(hyperopt): min_hold_bars=5 is hardcoded — candidate for hyperopt parameter. */ - if (prev_sign != 0 && curr_sign == 0 && hold_time < 5.0f && hold_time > 0.0f && !exiting_trade) { + /* ════════════════════════════════════════════════════════════════════ + * HOLD ENFORCEMENT: block ALL position changes during minimum hold. + * Applies to BOTH flat exits AND reversals. + * + * Replaces the old dead-code guard: + * `prev_sign != 0 && curr_sign == 0 && !exiting_trade` was always + * false because exiting_trade = (prev_sign != 0 && curr_sign == 0). + * + * Layer 2 safety net — Layer 1 (action masking) prevents most hold + * violations, but epsilon exploration can still slip through. + * ════════════════════════════════════════════════════════════════════ */ + 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 > 0.0f + && hold_time < (float)min_hold_bars + && (wants_exit || wants_reversal) + && bar_idx < total_bars - 1); /* bypass at episode end */ + + if (hold_violation) { + /* Override: keep previous position, cancel the action */ position = ps[0]; cash = ps[1]; - is_flat = 0.0f; - exiting_trade = 0; + is_flat = (fabsf(position) < 0.001f) ? 1.0f : 0.0f; + + /* Fix the stored action to match held exposure (prevent action aliasing) */ float prev_exposure_frac = ps[0] / (max_position > 0.0f ? max_position : 1.0f); - int held_exposure = (int)((prev_exposure_frac + 1.0f) / 0.25f + 0.5f); + int held_exposure = (int)roundf((prev_exposure_frac + 1.0f) * 0.5f * (float)(b0_size - 1)); if (held_exposure < 0) held_exposure = 0; if (held_exposure >= b0_size) held_exposure = b0_size - 1; int original_order = (action_idx / b2_size) % b1_size; int original_urgency = action_idx % b2_size; action_idx = held_exposure * b1_size * b2_size + original_order * b2_size + original_urgency; out_actions[out_off] = action_idx; + + /* Recompute signs from overridden position */ + curr_sign = prev_sign; + wants_exit = 0; + wants_reversal = 0; } - /* Recompute segment_complete after trailing stop / early-exit may have - * modified exiting_trade. Trailing stop sets exiting_trade=1; early-exit - * override sets exiting_trade=0. reversing_trade is never modified. */ - segment_complete = exiting_trade || reversing_trade; + /* Compute final trade lifecycle from (possibly overridden) state */ + exiting_trade = wants_exit; + reversing_trade = wants_reversal; + int segment_complete = exiting_trade || reversing_trade; /* Save hold_time BEFORE reset — sparse reward needs the pre-reset value * for the patience multiplier. Without this, hold_time is 0 at exit @@ -926,6 +944,15 @@ extern "C" __global__ void experience_env_step( reward = 10.0f * tanhf(reward / 10.0f); } + /* Layer 3: Reward scaling — shorter trades get proportionally less reward. + * Smooth gradient toward longer holds (vs binary cliff at min_hold). + * Floor at 10% to maintain gradient signal for edge cases. */ + if (segment_complete && segment_hold_time > 0.0f && min_hold_bars > 0) { + float hold_scale = fminf(segment_hold_time / (float)min_hold_bars, 1.0f); + hold_scale = fmaxf(hold_scale, 0.1f); + reward *= hold_scale; + } + /* Turnover penalty REMOVED (reward v6): tx costs already deducted from * cash at line ~679 (Almgren-Chriss impact model). The old 0.05*|delta| * penalty double-counted costs and over-penalized necessary rebalancing. */ diff --git a/crates/ml/src/cuda_pipeline/gpu_experience_collector.rs b/crates/ml/src/cuda_pipeline/gpu_experience_collector.rs index 9d5fccdcc..b221a2309 100644 --- a/crates/ml/src/cuda_pipeline/gpu_experience_collector.rs +++ b/crates/ml/src/cuda_pipeline/gpu_experience_collector.rs @@ -236,6 +236,8 @@ pub struct ExperienceCollectorConfig { pub n_steps: i32, /// Enable action masking: filters invalid exposure actions in the GPU kernel. pub enable_action_masking: bool, + /// Minimum bars to hold a position before exiting or reversing (Layer 2 hold enforcement). + pub min_hold_bars: i32, } impl Default for ExperienceCollectorConfig { @@ -283,6 +285,7 @@ impl Default for ExperienceCollectorConfig { dsr_eta: 0.01, n_steps: 1, enable_action_masking: false, + min_hold_bars: 5, } } } @@ -1291,6 +1294,7 @@ impl GpuExperienceCollector { // ── 5. Environment step (reward v5: trade-aware hybrid) ────── let max_pos = config.max_position; + let min_hold_bars_i32 = config.min_hold_bars; let tx_cost = config.tx_cost_multiplier; let rw_loss_av = config.loss_aversion; let l_i32 = timesteps as i32; @@ -1324,6 +1328,7 @@ impl GpuExperienceCollector { .arg(&self.cvar_scales_ptr) // CVaR position scaling (0 = NULL = no scaling) .arg(&self.q_gaps_buf) // Q-gap conviction scaling .arg(&mut self.raw_returns_out) // Raw portfolio returns (unshaped) for Sharpe/MaxDD + .arg(&min_hold_bars_i32) // min_hold_bars for hold enforcement .launch(launch_cfg) .map_err(|e| MLError::ModelError(format!( "experience_env_step t={t}: {e}" diff --git a/crates/ml/src/trainers/dqn/trainer/training_loop.rs b/crates/ml/src/trainers/dqn/trainer/training_loop.rs index 0ba6fe3ff..b404a678d 100644 --- a/crates/ml/src/trainers/dqn/trainer/training_loop.rs +++ b/crates/ml/src/trainers/dqn/trainer/training_loop.rs @@ -1049,6 +1049,7 @@ impl DQNTrainer { }, dsr_eta: self.hyperparams.dsr_eta as f32, n_steps: self.hyperparams.n_steps as i32, + min_hold_bars: self.hyperparams.min_hold_bars as i32, ..Default::default() };