fix(ml): update PPO trainer reward and position tracking for 45 factored actions
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 <noreply@anthropic.com>
This commit is contained in:
@@ -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);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user