The 4-branch DQN (direction x magnitude) had 3 degenerate variants (Short25, Flat, Long25) that all mapped to 0.0 target exposure when direction=Flat, causing 82% Flat collapse. Collapse these into a single Flat variant, giving 7 levels (ShortSmall/Half/Full, Flat, LongSmall/Half/Full) and 63 total factored actions (7x3x3). - ExposureLevel enum: 9 variants -> 7 (add direction/magnitude/from_dir_mag) - FactoredAction: 81 -> 63 total actions, from_index/to_index updated - DQN epsilon-greedy: use from_dir_mag() instead of dir*3+mag indexing - DQN config: num_actions default 9 -> 7 - PPO action space: 45 -> 63 actions, action masking updated - Signal adapter CUDA kernel: 5-bin -> 7-bin exposure aggregation - All tests updated for new variant names and index ranges Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
297 lines
11 KiB
Rust
297 lines
11 KiB
Rust
#![allow(
|
|
clippy::assertions_on_constants,
|
|
clippy::assertions_on_result_states,
|
|
clippy::clone_on_copy,
|
|
clippy::decimal_literal_representation,
|
|
clippy::doc_markdown,
|
|
clippy::empty_line_after_doc_comments,
|
|
clippy::field_reassign_with_default,
|
|
clippy::get_unwrap,
|
|
clippy::identity_op,
|
|
clippy::inconsistent_digit_grouping,
|
|
clippy::indexing_slicing,
|
|
clippy::integer_division,
|
|
clippy::len_zero,
|
|
clippy::let_underscore_must_use,
|
|
clippy::manual_div_ceil,
|
|
clippy::manual_let_else,
|
|
clippy::manual_range_contains,
|
|
clippy::modulo_arithmetic,
|
|
clippy::needless_range_loop,
|
|
clippy::non_ascii_literal,
|
|
clippy::redundant_clone,
|
|
clippy::shadow_reuse,
|
|
clippy::shadow_same,
|
|
clippy::shadow_unrelated,
|
|
clippy::single_match_else,
|
|
clippy::str_to_string,
|
|
clippy::string_slice,
|
|
clippy::tests_outside_test_module,
|
|
clippy::too_many_lines,
|
|
clippy::unnecessary_wraps,
|
|
clippy::unseparated_literal_suffix,
|
|
clippy::use_debug,
|
|
clippy::useless_vec,
|
|
clippy::wildcard_enum_match_arm,
|
|
clippy::else_if_without_else,
|
|
clippy::expect_used,
|
|
clippy::missing_const_for_fn,
|
|
clippy::similar_names,
|
|
clippy::type_complexity,
|
|
clippy::collapsible_else_if,
|
|
clippy::doc_lazy_continuation,
|
|
clippy::items_after_test_module,
|
|
clippy::map_clone,
|
|
clippy::multiple_unsafe_ops_per_block,
|
|
clippy::unwrap_or_default,
|
|
clippy::assign_op_pattern,
|
|
clippy::needless_borrow,
|
|
clippy::println_empty_string,
|
|
clippy::unnecessary_cast,
|
|
clippy::used_underscore_binding,
|
|
clippy::create_dir,
|
|
clippy::implicit_saturating_sub,
|
|
clippy::exit,
|
|
clippy::expect_fun_call,
|
|
clippy::too_many_arguments,
|
|
clippy::unnecessary_map_or,
|
|
clippy::unwrap_used,
|
|
dead_code,
|
|
unused_imports,
|
|
unused_variables,
|
|
clippy::cloned_ref_to_slice_refs,
|
|
clippy::neg_multiply,
|
|
clippy::while_let_loop,
|
|
clippy::bool_assert_comparison,
|
|
clippy::excessive_precision,
|
|
clippy::trivially_copy_pass_by_ref,
|
|
clippy::op_ref,
|
|
clippy::redundant_closure,
|
|
clippy::unnecessary_lazy_evaluations,
|
|
clippy::if_then_some_else_none,
|
|
clippy::unnecessary_to_owned,
|
|
clippy::single_component_path_imports,
|
|
)]
|
|
//! Regression tests for Bugs #21, #22, #23 - Compilation Error Prevention
|
|
//!
|
|
//! These tests ensure that:
|
|
//! 1. Bug #21: Decimal conversion fallback returns Decimal, not ()
|
|
//! 2. Bug #22: PortfolioTracker has high_water_mark() method if needed
|
|
//! 3. Bug #23: unrealized_pnl() is always called with current_price argument
|
|
//!
|
|
//! Status: All bugs already fixed in codebase (2025-11-14)
|
|
//! Purpose: Regression prevention for future refactoring
|
|
|
|
use ml::dqn::portfolio_tracker::PortfolioTracker;
|
|
use tracing::info;
|
|
|
|
/// Test 1: Bug #21 - Decimal conversion fallback returns previous value (Decimal), not ()
|
|
///
|
|
/// This test verifies that Decimal conversion fallback logic compiles correctly.
|
|
/// The actual implementation is in ml/src/dqn/risk_integration.rs (internal module).
|
|
///
|
|
/// Bug #21 (ALREADY FIXED):
|
|
/// - Location: ml/src/dqn/risk_integration.rs:48-52
|
|
/// - Issue: unwrap_or_else() closure must return Decimal, not ()
|
|
/// - Fix: Returns *self.portfolio_value.blocking_read() (Decimal)
|
|
///
|
|
/// NOTE: risk_integration module is internal and not exported, so we test the pattern
|
|
/// through PortfolioTracker which uses similar conversion patterns.
|
|
#[test]
|
|
fn test_decimal_conversion_pattern_compiles() {
|
|
use rust_decimal::Decimal;
|
|
|
|
// Test the Decimal conversion pattern that Bug #21 refers to
|
|
let test_value = 100_000.0;
|
|
let previous_value = Decimal::new(95_000, 0);
|
|
|
|
// This is the pattern that Bug #21 fixed:
|
|
// unwrap_or_else() must return Decimal, not ()
|
|
let value_decimal = Decimal::try_from(test_value).unwrap_or_else(|_| {
|
|
// CORRECT: Returns Decimal (previous value)
|
|
previous_value
|
|
});
|
|
|
|
assert_eq!(value_decimal, Decimal::new(100_000, 0));
|
|
|
|
// Test with NaN (should fall back to previous value)
|
|
let nan_value = f64::NAN;
|
|
let fallback_decimal = Decimal::try_from(nan_value).unwrap_or_else(|_| {
|
|
// CORRECT: Returns Decimal (previous value), not ()
|
|
previous_value
|
|
});
|
|
|
|
assert_eq!(fallback_decimal, Decimal::new(95_000, 0));
|
|
|
|
info!("Bug #21 Prevention: Decimal conversion fallback compiles correctly");
|
|
}
|
|
|
|
/// Test 2: Bug #22 - PortfolioTracker high_water_mark() method exists (if needed)
|
|
///
|
|
/// This test verifies that PortfolioTracker has all necessary accessor methods
|
|
/// for portfolio metrics including high_water_mark() if required.
|
|
///
|
|
/// Bug #22 (DOESN'T EXIST):
|
|
/// - Location: ml/src/trainers/dqn.rs:1091 (claimed)
|
|
/// - Issue: Method not found on PortfolioTracker
|
|
/// - Status: high_water_mark() is never called in codebase
|
|
/// - Purpose: This test documents that the method isn't needed
|
|
#[test]
|
|
fn test_portfolio_tracker_accessor_methods() {
|
|
let tracker = PortfolioTracker::new(100_000.0, 0.0001, 0.0);
|
|
|
|
// Verify existing accessor methods work
|
|
let cash = tracker.cash_balance();
|
|
assert_eq!(cash, 100_000.0);
|
|
|
|
let position = tracker.current_position();
|
|
assert_eq!(position, 0.0);
|
|
|
|
let total = tracker.total_value(5000.0);
|
|
assert_eq!(total, 100_000.0);
|
|
|
|
let entry_price = tracker.average_entry_price();
|
|
assert_eq!(entry_price, 0.0);
|
|
|
|
let realized = tracker.realized_pnl();
|
|
assert_eq!(realized, 0.0);
|
|
|
|
let unrealized = tracker.unrealized_pnl(5000.0); // ✅ Takes current_price argument
|
|
assert_eq!(unrealized, 0.0);
|
|
|
|
let tx_costs = tracker.transaction_costs();
|
|
assert_eq!(tx_costs, 0.0);
|
|
|
|
// NOTE: high_water_mark() method is NOT needed in current codebase
|
|
// If it's added in the future, this test should be updated to verify it
|
|
|
|
info!("Bug #22 Prevention: All PortfolioTracker accessor methods work");
|
|
}
|
|
|
|
/// Test 3: Bug #23 - unrealized_pnl() accepts current_price argument
|
|
///
|
|
/// This test verifies that PortfolioTracker::unrealized_pnl() correctly requires
|
|
/// and uses the current_price argument for accurate P&L calculations.
|
|
///
|
|
/// Bug #23 (DOESN'T EXIST):
|
|
/// - Location: ml/src/trainers/dqn.rs:1098, 1100 (claimed)
|
|
/// - Issue: unrealized_pnl() called without current_price argument
|
|
/// - Status: Method signature requires current_price: f32
|
|
/// - Purpose: Verify correct usage pattern
|
|
#[test]
|
|
fn test_unrealized_pnl_with_current_price() {
|
|
let mut tracker = PortfolioTracker::new(100_000.0, 0.0001, 0.0);
|
|
|
|
// Open a long position at $5000
|
|
use ml::dqn::action_space::{FactoredAction, ExposureLevel, OrderType, Urgency};
|
|
let action = FactoredAction::new(ExposureLevel::LongFull, OrderType::Market, Urgency::Normal);
|
|
tracker.execute_action(action, 5000.0, 10.0); // 10 contracts at $5000
|
|
|
|
// Verify position opened
|
|
assert_eq!(tracker.current_position(), 10.0);
|
|
|
|
// Calculate unrealized P&L at different prices
|
|
let pnl_at_entry = tracker.unrealized_pnl(5000.0); // ✅ Must pass current_price
|
|
assert!((pnl_at_entry - 0.0).abs() < 100.0); // Near zero at entry price (minus tx costs)
|
|
|
|
let pnl_at_5100 = tracker.unrealized_pnl(5100.0); // ✅ Must pass current_price
|
|
assert!(pnl_at_5100 > 0.0, "Price increase should generate profit");
|
|
|
|
let pnl_at_4900 = tracker.unrealized_pnl(4900.0); // ✅ Must pass current_price
|
|
assert!(pnl_at_4900 < 0.0, "Price decrease should generate loss");
|
|
|
|
// Verify P&L calculations are reasonable
|
|
// Position: 10 contracts, Price change: +$100 = +$1000 profit (minus tx costs)
|
|
assert!((pnl_at_5100 - 1000.0).abs() < 100.0, "Expected ~$1000 profit at $5100");
|
|
|
|
// Position: 10 contracts, Price change: -$100 = -$1000 loss (minus tx costs)
|
|
assert!((pnl_at_4900 + 1000.0).abs() < 100.0, "Expected ~$1000 loss at $4900");
|
|
|
|
info!("Bug #23 Prevention: unrealized_pnl() requires current_price argument");
|
|
}
|
|
|
|
/// Test 4: Comprehensive portfolio tracker integration test
|
|
///
|
|
/// This test verifies that all portfolio tracking methods work correctly together,
|
|
/// ensuring that any future refactoring won't break the core functionality.
|
|
#[test]
|
|
fn test_portfolio_tracker_comprehensive_integration() {
|
|
let mut tracker = PortfolioTracker::new(100_000.0, 0.0001, 0.0);
|
|
|
|
use ml::dqn::action_space::{FactoredAction, ExposureLevel, OrderType, Urgency};
|
|
|
|
// Test 1: Open long position
|
|
let long_action = FactoredAction::new(ExposureLevel::LongSmall, OrderType::LimitMaker, Urgency::Normal);
|
|
tracker.execute_action(long_action, 5000.0, 10.0); // 5 contracts (50% of 10 max)
|
|
|
|
assert_eq!(tracker.current_position(), 5.0);
|
|
let unrealized_5000 = tracker.unrealized_pnl(5000.0); // ✅ Current price required
|
|
assert!(unrealized_5000.abs() < 50.0); // Near zero (minus small tx costs)
|
|
|
|
// Test 2: Price moves up - check unrealized P&L
|
|
let unrealized_5200 = tracker.unrealized_pnl(5200.0); // ✅ Current price required
|
|
assert!(unrealized_5200 > 0.0, "Should have unrealized profit");
|
|
|
|
// Test 3: Close position - check realized P&L
|
|
let flat_action = FactoredAction::new(ExposureLevel::Flat, OrderType::Market, Urgency::Aggressive);
|
|
tracker.execute_action(flat_action, 5200.0, 10.0);
|
|
|
|
assert_eq!(tracker.current_position(), 0.0);
|
|
let realized = tracker.realized_pnl();
|
|
assert!(realized > 0.0, "Should have realized profit after closing winning position");
|
|
|
|
// Test 4: All accessor methods return valid values
|
|
assert!(tracker.cash_balance().is_finite());
|
|
assert_eq!(tracker.current_position(), 0.0);
|
|
assert!(tracker.total_value(5200.0).is_finite());
|
|
assert_eq!(tracker.average_entry_price(), 0.0); // No position
|
|
assert!(tracker.realized_pnl().is_finite());
|
|
|
|
// Unrealized P&L when flat should be close to zero (may have small rounding)
|
|
let unrealized = tracker.unrealized_pnl(5200.0); // ✅ No position = no unrealized P&L
|
|
assert!(unrealized.abs() < 1000.0, "Unrealized P&L should be near zero when flat, got: {}", unrealized);
|
|
|
|
assert!(tracker.transaction_costs() > 0.0); // Should have incurred tx costs
|
|
|
|
info!("Comprehensive Integration: All portfolio tracking methods work correctly");
|
|
}
|
|
|
|
/// Test 5: Rust_decimal conversion edge cases
|
|
///
|
|
/// This test verifies that Decimal::try_from() handles edge cases correctly,
|
|
/// which is relevant to Bug #21's context (conversion fallback logic).
|
|
#[test]
|
|
fn test_decimal_conversion_edge_cases() {
|
|
use rust_decimal::Decimal;
|
|
|
|
// Test normal values
|
|
let normal = 100_000.0;
|
|
assert!(Decimal::try_from(normal).is_ok());
|
|
|
|
// Test zero
|
|
let zero = 0.0;
|
|
assert_eq!(Decimal::try_from(zero).unwrap(), Decimal::ZERO);
|
|
|
|
// Test negative
|
|
let negative = -5000.0;
|
|
assert!(Decimal::try_from(negative).is_ok());
|
|
|
|
// Test very large value
|
|
let large = 1_000_000_000.0;
|
|
assert!(Decimal::try_from(large).is_ok());
|
|
|
|
// Test NaN (should fail)
|
|
let nan = f64::NAN;
|
|
assert!(Decimal::try_from(nan).is_err(), "NaN should fail conversion");
|
|
|
|
// Test infinity (should fail)
|
|
let inf = f64::INFINITY;
|
|
assert!(Decimal::try_from(inf).is_err(), "Infinity should fail conversion");
|
|
|
|
// Test negative infinity (should fail)
|
|
let neg_inf = f64::NEG_INFINITY;
|
|
assert!(Decimal::try_from(neg_inf).is_err(), "Negative infinity should fail conversion");
|
|
|
|
info!("Bug #21 Context: Decimal edge cases handled correctly");
|
|
}
|