From 82b52a0230f3d1929dc902e32f329796dfb71774 Mon Sep 17 00:00:00 2001 From: jgrusewski Date: Thu, 12 Mar 2026 17:14:08 +0100 Subject: [PATCH] =?UTF-8?q?fix(dqn):=20walk-forward=20feature=20order=20mi?= =?UTF-8?q?smatch=20=E2=80=94=20portfolio=20at=20correct=20indices?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The gather kernel places live portfolio features at [feat_dim..feat_dim+3]. Training builds states as [market(42), portfolio(3), OFI(8)] with portfolio at indices 42-44. The hyperopt adapter was setting feat_dim=50 (raw_state_dim minus 3), which placed live portfolio at indices 50-52 — invisible to the model. The model saw zeros for position/value/spread during walk-forward, making random decisions and producing -775% return with 0% win rate. Fix: Pass only 42 market features (strip portfolio zeros from val_data). For OFI-enabled models, pad to 50 and shuffle the state tensor before forward pass to maintain [market, portfolio, OFI, pad] order. Also fixes pre-existing clippy warnings in ml-dqn (doc_markdown, cognitive_complexity). Co-Authored-By: Claude Opus 4.6 --- crates/ml-dqn/src/dqn.rs | 3 +- crates/ml/src/hyperopt/adapters/dqn.rs | 72 +++++++++++++++++++++----- 2 files changed, 61 insertions(+), 14 deletions(-) diff --git a/crates/ml-dqn/src/dqn.rs b/crates/ml-dqn/src/dqn.rs index 9ffd07028..e4b96955a 100644 --- a/crates/ml-dqn/src/dqn.rs +++ b/crates/ml-dqn/src/dqn.rs @@ -1590,6 +1590,7 @@ impl DQN { } /// Select action using epsilon-greedy policy + #[allow(clippy::cognitive_complexity)] pub fn select_action(&mut self, state: &[f32]) -> Result { // Reset noise for exploration (Noisy Networks / Rainbow DQN) if self.config.use_noisy_nets { @@ -4227,7 +4228,7 @@ impl DQN { /// Get the effective epsilon for exploration. /// /// With noisy nets enabled, maintains a minimum epsilon floor to prevent - /// action collapse. NoisyNets provide learned exploration but their noise + /// action collapse. `NoisyNets` provide learned exploration but their noise /// magnitude can become smaller than the Q-value gap between actions, /// causing the agent to converge to a single action (e.g., Short100 only). /// The floor ensures at least 10% random actions (2% per exposure level). diff --git a/crates/ml/src/hyperopt/adapters/dqn.rs b/crates/ml/src/hyperopt/adapters/dqn.rs index 488aa399f..c2e6d5723 100644 --- a/crates/ml/src/hyperopt/adapters/dqn.rs +++ b/crates/ml/src/hyperopt/adapters/dqn.rs @@ -1667,6 +1667,18 @@ impl DQNTrainer { let val_data = internal_trainer.get_val_data(); + // FIX: The gather kernel places live portfolio at [feat_dim..feat_dim+3]. + // Training builds states as [market(42), portfolio(3), OFI(8)], so portfolio + // is at indices 42-44. We must pass only market features (42 dims) as the + // "feature" part so the kernel inserts portfolio at exactly index 42. + // + // For OFI-enabled models, we pad features to 50 (42 market + 8 OFI placeholder) + // so state_dim = (50+3+7)&!7 = 56 (matching training). The forward closure + // then shuffles [market,ofi_pad,portfolio,pad] → [market,portfolio,ofi_pad,pad]. + let ofi_enabled = self.mbp10_data_dir.is_some(); + let market_dim: usize = 42; + let feature_dim: usize = if ofi_enabled { 50 } else { market_dim }; + let mut window_prices = Vec::with_capacity(window_count); let mut window_features = Vec::with_capacity(window_count); @@ -1690,17 +1702,25 @@ impl DQNTrainer { val_data.len() )) })?; - let fv_f32: Vec = fv.iter().map(|&v| v as f32).collect(); + // Take only the 42 market features, strip portfolio zeros at 42-44. + let fv_slice = fv.get(..market_dim).ok_or_else(|| { + MLError::ConfigError(format!( + "feature vector too short: {} < {market_dim}", + fv.len() + )) + })?; + let mut fv_f32: Vec = fv_slice.iter().map(|&v| v as f32).collect(); + // For OFI models, zero-pad from 42 to 50. The model was trained with + // real OFI at 45-52 but zeros are the safe fallback (matches the + // `ofi_enabled && data_missing` path in feature_vector_to_state_with_ofi). + if ofi_enabled { + fv_f32.resize(feature_dim, 0.0); + } features.push(fv_f32); } window_prices.push(prices); window_features.push(features); } - - // Market features only -- portfolio features (3) added by evaluator's gather_states. - // raw_state_dim = 53 (with OFI) or 45 (without), subtract 3 portfolio dims. - let raw_state_dim: usize = if self.mbp10_data_dir.is_some() { 53 } else { 45 }; - let feature_dim = raw_state_dim - 3; let config = GpuBacktestConfig { max_position: 1.0, tx_cost_bps: self.tx_cost_bps as f32, @@ -1728,13 +1748,39 @@ impl DQNTrainer { // TODO: Switch to evaluator.evaluate_dqn() with DuelingWeightSet extraction // when the InternalDQNTrainer exposes the dueling VarMap. The closure path // through agent_guard.batch_q_values() still works but has candle dispatch overhead. - let metrics = evaluator.evaluate( - &|states: &candle_core::Tensor| -> Result { - agent_guard.batch_q_values(states) - }, - 3, // portfolio_dim - device, - )?; + let metrics = if ofi_enabled { + // OFI case: kernel produces [market(42), ofi_pad(8), portfolio(3), pad(3)] = 56. + // Model expects [market(42), portfolio(3), OFI(8), pad(3)] = 56. + // Shuffle the state tensor to place portfolio at the correct indices. + evaluator.evaluate( + &|states: &candle_core::Tensor| -> Result { + let market = states.narrow(1, 0, 42) + .map_err(|e| MLError::ModelError(format!("narrow market: {e}")))?; + let ofi_block = states.narrow(1, 42, 8) + .map_err(|e| MLError::ModelError(format!("narrow ofi: {e}")))?; + let portfolio = states.narrow(1, 50, 3) + .map_err(|e| MLError::ModelError(format!("narrow portfolio: {e}")))?; + let padding = states.narrow(1, 53, 3) + .map_err(|e| MLError::ModelError(format!("narrow padding: {e}")))?; + let shuffled = candle_core::Tensor::cat( + &[&market, &portfolio, &ofi_block, &padding], 1, + ).map_err(|e| MLError::ModelError(format!("cat shuffle: {e}")))?; + agent_guard.batch_q_values(&shuffled) + }, + 3, // portfolio_dim + device, + )? + } else { + // Non-OFI: kernel produces [market(42), portfolio(3), pad(3)] = 48. + // Matches training layout exactly — no shuffle needed. + evaluator.evaluate( + &|states: &candle_core::Tensor| -> Result { + agent_guard.batch_q_values(states) + }, + 3, // portfolio_dim + device, + )? + }; drop(agent_guard);