From 18f98e17bb52618f498ad05f4635e0bc12555eb8 Mon Sep 17 00:00:00 2001 From: jgrusewski Date: Mon, 2 Mar 2026 15:29:28 +0100 Subject: [PATCH] fix(ml): update PPO trainer reward and position tracking for 45 factored actions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit compute_reward_pnl() took raw action_idx (0=Buy,1=Sell,2=Hold) — wrong with 45-action FactoredAction encoding. Now takes &FactoredAction and uses order-type-specific transaction costs. Position tracking uses action.exposure instead of old 3-way index match. Co-Authored-By: Claude Opus 4.6 --- crates/ml/src/trainers/ppo.rs | 71 ++++++++++++++++++----------------- 1 file changed, 36 insertions(+), 35 deletions(-) diff --git a/crates/ml/src/trainers/ppo.rs b/crates/ml/src/trainers/ppo.rs index 7a6f41772..6888cc722 100644 --- a/crates/ml/src/trainers/ppo.rs +++ b/crates/ml/src/trainers/ppo.rs @@ -844,13 +844,15 @@ impl PpoTrainer { // Compute reward based on actual PnL // Get log return from state (last element, safe indexing) let log_return = state.last().copied().unwrap_or(0.0); - let reward = self.compute_reward_pnl(action_idx, log_return, position); + let reward = self.compute_reward_pnl(&action, log_return, position); - // Update position based on action - position = match action_idx { - 0 => 1, // Buy -> Long position - 1 => -1, // Sell -> Short position - _ => 0, // Hold -> Neutral + // Update position based on factored action exposure + position = match action.exposure { + crate::common::action::ExposureLevel::Long100 => 1, + crate::common::action::ExposureLevel::Long50 => 1, + crate::common::action::ExposureLevel::Flat => 0, + crate::common::action::ExposureLevel::Short50 => -1, + crate::common::action::ExposureLevel::Short100 => -1, }; let done = step_idx == num_steps - 1; @@ -966,12 +968,14 @@ impl PpoTrainer { step_count += 1; let log_return = state.last().copied().unwrap_or(0.0); - let reward = self.compute_reward_pnl(action_idx, log_return, position); + let reward = self.compute_reward_pnl(&action, log_return, position); - position = match action_idx { - 0 => 1, - 1 => -1, - _ => 0, + position = match action.exposure { + crate::common::action::ExposureLevel::Long100 => 1, + crate::common::action::ExposureLevel::Long50 => 1, + crate::common::action::ExposureLevel::Flat => 0, + crate::common::action::ExposureLevel::Short50 => -1, + crate::common::action::ExposureLevel::Short100 => -1, }; let done = step_idx == end_idx - 1; @@ -1384,7 +1388,7 @@ impl PpoTrainer { /// - Short position: reward = -log_return (profit when price decreases) /// - Neutral: reward = 0 (no exposure) /// - Sharpe ratio bonus: small bonus for consistent returns - fn compute_reward_pnl(&self, action_idx: usize, log_return: f32, current_position: i8) -> f32 { + fn compute_reward_pnl(&self, action: &FactoredAction, log_return: f32, current_position: i8) -> f32 { // Base PnL reward from position and market movement let pnl_reward = match current_position { 1 => log_return * 1000.0, // Long: profit when price goes up (scaled to ±0.1 range) @@ -1392,21 +1396,12 @@ impl PpoTrainer { _ => 0.0, // Neutral: no exposure }; - // Action-specific penalties/bonuses - let action_modifier = match action_idx { - 0 => { - // Buy action: small penalty for trading costs - -0.0001 - }, - 1 => { - // Sell action: small penalty for trading costs - -0.0001 - }, - 2 => { - // Hold action: no trading cost - 0.0 - }, - _ => 0.0, + // Action-specific transaction cost penalty from order type + let action_modifier = if action.is_hold() { + 0.0 // Flat exposure: no trading cost + } else { + // Non-hold actions incur order-type-specific transaction costs + -(action.order.cost_bps() / 10000.0) * 0.1 // Scale cost to reward magnitude }; // Sharpe ratio bonus: reward consistent positive returns @@ -1697,33 +1692,39 @@ mod tests { #[test] fn test_reward_computation() { + use crate::common::action::{ExposureLevel, OrderType, Urgency}; + let params = create_test_params(); let trainer = PpoTrainer::new(params, 64, "/tmp/ppo_checkpoints", false, None).unwrap(); + let hold = FactoredAction::new(ExposureLevel::Flat, OrderType::Market, Urgency::Normal); + let buy = FactoredAction::new(ExposureLevel::Long100, OrderType::Market, Urgency::Normal); + let sell = FactoredAction::new(ExposureLevel::Short100, OrderType::Market, Urgency::Normal); + // Test 1: Long position with positive return should be profitable - let reward_long_up = trainer.compute_reward_pnl(2, 0.01, 1); // Hold with long position, market up - let reward_neutral = trainer.compute_reward_pnl(2, 0.01, 0); // Hold with neutral position + let reward_long_up = trainer.compute_reward_pnl(&hold, 0.01, 1); // Hold with long position, market up + let reward_neutral = trainer.compute_reward_pnl(&hold, 0.01, 0); // Hold with neutral position // Long position captures positive return assert!(reward_long_up > reward_neutral); assert!(reward_long_up > 0.0); // Test 2: Hold should avoid trading costs compared to buy/sell - let reward_buy = trainer.compute_reward_pnl(0, 0.01, 1); // Buy with long position - let reward_sell = trainer.compute_reward_pnl(1, 0.01, 1); // Sell with long position - let reward_hold = trainer.compute_reward_pnl(2, 0.01, 1); // Hold with long position + let reward_buy = trainer.compute_reward_pnl(&buy, 0.01, 1); // Buy with long position + let reward_sell = trainer.compute_reward_pnl(&sell, 0.01, 1); // Sell with long position + let reward_hold = trainer.compute_reward_pnl(&hold, 0.01, 1); // Hold with long position // Hold should be better than buy/sell when already positioned (avoids trading costs) assert!(reward_hold > reward_buy); assert!(reward_hold > reward_sell); // Test 3: Short position with negative return should be profitable - let reward_short_down = trainer.compute_reward_pnl(2, -0.01, -1); // Hold with short position, market down + let reward_short_down = trainer.compute_reward_pnl(&hold, -0.01, -1); // Hold with short position, market down assert!(reward_short_down > 0.0); // Test 4: Wrong-way positions should have penalties - let reward_long_down = trainer.compute_reward_pnl(2, -0.01, 1); // Long position, market down - let reward_short_up = trainer.compute_reward_pnl(2, 0.01, -1); // Short position, market up + let reward_long_down = trainer.compute_reward_pnl(&hold, -0.01, 1); // Long position, market down + let reward_short_up = trainer.compute_reward_pnl(&hold, 0.01, -1); // Short position, market up assert!(reward_long_down < 0.0); assert!(reward_short_up < 0.0); }