diff --git a/CHANGELOG.md b/CHANGELOG.md index afecf47..6fa9fb7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,12 @@ API/platform compatibility policy, and a prioritized FX roadmap. ### Fixed +- Python constructors reject nonfinite or out-of-range broker settings. Order, + partial-close, stop-update, closing-price, and equity-price inputs are checked + before account mutation. Finite nonpositive partial sizes remain no-ops. +- Optimization rejects malformed or nonfinite signals, including direct + `run_grid()` calls. Native SMA calls reject invalid sizes and windows before + simulation rather than risking a Rust panic. - SL/TP processing checks the midpoint open first: crossed stops fill at the adverse open, while crossed targets receive the favorable open, before any later range touches. Exit spread, commission, and conversion still apply. @@ -24,6 +30,11 @@ the exit-side spread (future exit commission is still charged only at closing). ### Migration +- Python cash, contract size, conversion rate, opening sizes, prices, and supplied + SL/TP levels must be positive and finite; spread and commission must be finite + and nonnegative. Invalid numeric inputs now raise `ValueError`. Signal arrays + must be one-dimensional, finite, and contain one target per bar. See README + numeric input validation for partial-close and native SMA rules. - Gap-bar SL/TP fills now use the opening price instead of the configured level. An opening target also takes precedence over a later stop touch. Rerun affected backtests: PnL, equity, and strategy decisions can change; there is no legacy diff --git a/README.md b/README.md index 9a10e37..60d170e 100644 --- a/README.md +++ b/README.md @@ -195,6 +195,46 @@ Backtest( ) ``` +### Numeric input validation + +`Backtest`, `Broker`, and `Engine` constructors reject invalid numeric settings +with `ValueError` before simulation: + +| Input | Allowed values | +| --- | --- | +| `cash` (`initial_cash` on `Broker` and `Engine`) | Finite and greater than zero | +| `commission`, `spread` | Finite and nonnegative | +| `contract_size`, `quote_to_account` | Finite and greater than zero | + +Python `buy()` and `sell()` require positive finite lot sizes and prices. +Supplied stop-loss and take-profit levels, including `update_sl()` values, must +also be positive and finite. Levels are not restricted relative to the entry +price: a trailing stop can lock in profit, and stops can still be widened. +Closing prices and equity mark prices must be positive and finite. +Rejected calls leave cash, positions, and position IDs unchanged. + +`close_partial()` rejects nonfinite sizes. Finite zero or negative sizes remain +no-ops, and a size at or above the remaining position closes it completely. +Unknown position IDs remain no-ops; `update_sl()` returns `False` for them. +These behaviors require otherwise valid numeric arguments. + +`optimize()` requires each signal to be a one-dimensional numeric array with +one finite target lot size per bar. Positive targets mean long, negative targets +mean short, and zero means flat. Use zero during indicator warmup. Both NaN and +infinity are rejected. Direct extension `run_grid()` calls also check every +signal's length and finiteness before starting worker threads. +`Engine.run_native_sma()` requires a positive finite lot size and integer +windows satisfying `1 <= fast <= slow`. + +These checks apply at Python entry points, including direct extension calls. +Native Rust callers remain responsible for supplying valid inputs to the +simulation methods. OHLC data and timestamp validation are tracked separately +in [#15](https://github.com/KhizarImran/backtestingfx/issues/15). + +**Migration:** replace invalid inputs that previously passed through, including +zero initial cash, nonpositive opening sizes, and infinite signals. Calls now +raise an error instead of silently producing invalid account state. + ### Spread and price conventions OHLC prices are **midpoints** between bid and ask. `spread` is the constant full diff --git a/ROADMAP.md b/ROADMAP.md index 1c9b8cc..ea7ee06 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -16,7 +16,8 @@ priorities or delivered capabilities change. - Implemented (unreleased): gap-aware stop/target fills ([#8](https://github.com/KhizarImran/backtestingfx/issues/8)); see README execution policy. - Data and timestamp validation: [#15](https://github.com/KhizarImran/backtestingfx/issues/15). -- Broker, size, and signal validation: [#16](https://github.com/KhizarImran/backtestingfx/issues/16). +- Implemented (unreleased): Python broker, size, and signal validation + ([#16](https://github.com/KhizarImran/backtestingfx/issues/16)). - Independent native engine runs: [#18](https://github.com/KhizarImran/backtestingfx/issues/18). ## 2. Core adoption (P1) diff --git a/backtestingfx/backtest.py b/backtestingfx/backtest.py index d6a75d5..f852cc7 100644 --- a/backtestingfx/backtest.py +++ b/backtestingfx/backtest.py @@ -132,6 +132,7 @@ class Backtest: contract_size=100000.0, quote_to_account=1.0, ): + _rust._validate_config(cash, commission, spread, contract_size, quote_to_account) self._df = df self._strategy_class = strategy_class self._cash = cash @@ -218,11 +219,20 @@ class Backtest: signals = [] for combo in combos: signal = np.asarray(signal_fn(self._df, **combo), dtype=float) + if signal.ndim != 1: + raise ValueError(f"signal_fn must return a one-dimensional signal for {combo}") + if len(signal) != len(self._df): + raise ValueError( + f"signal_fn returned {len(signal)} values for {combo}, expected " + f"{len(self._df)} (one per bar)" + ) if np.isnan(signal).any(): raise ValueError( f"signal_fn returned NaN for {combo} — indicator warmup should " "produce 0.0 (flat), not NaN" ) + if not np.isfinite(signal).all(): + raise ValueError(f"signal_fn must return finite values for {combo}") # ponytail: tolist() is the cheap bridge into Rust. It costs one Python # float per bar per combo; swap in the `numpy` crate for a zero-copy # PyReadonlyArray1 if this ever shows up in a profile. diff --git a/src/broker.rs b/src/broker.rs index f254fbc..a2d53a1 100644 --- a/src/broker.rs +++ b/src/broker.rs @@ -1,4 +1,5 @@ use crate::types::{Bar, Position, Trade}; +use crate::validation; use pyo3::prelude::*; #[pyclass] @@ -114,9 +115,7 @@ impl Broker { } } -#[pymethods] impl Broker { - #[new] pub fn new( initial_cash: f64, commission: f64, @@ -267,6 +266,89 @@ impl Broker { } } +// Python inputs are checked before the native methods can mutate account state. +#[pymethods] +impl Broker { + #[new] + fn py_new( + initial_cash: f64, + commission: f64, + spread: f64, + contract_size: f64, + quote_to_account: f64, + ) -> PyResult { + validation::config(initial_cash, commission, spread, contract_size, quote_to_account)?; + Ok(Self::new(initial_cash, commission, spread, contract_size, quote_to_account)) + } + + #[pyo3(name = "positions")] + fn py_positions(&self) -> Vec { + self.positions() + } + + #[pyo3(name = "buy")] + fn py_buy( + &mut self, + price: f64, + lot_size: f64, + timestamp: i64, + stop_loss: Option, + take_profit: Option, + ) -> PyResult<()> { + validation::order(price, lot_size, stop_loss, take_profit)?; + self.buy(price, lot_size, timestamp, stop_loss, take_profit); + Ok(()) + } + + #[pyo3(name = "sell")] + fn py_sell( + &mut self, + price: f64, + lot_size: f64, + timestamp: i64, + stop_loss: Option, + take_profit: Option, + ) -> PyResult<()> { + validation::order(price, lot_size, stop_loss, take_profit)?; + self.sell(price, lot_size, timestamp, stop_loss, take_profit); + Ok(()) + } + + #[pyo3(name = "close_position")] + fn py_close_position(&mut self, id: u64, price: f64, timestamp: i64) -> PyResult<()> { + validation::positive("price", price)?; + self.close_position(id, price, timestamp); + Ok(()) + } + + #[pyo3(name = "close_partial")] + fn py_close_partial(&mut self, id: u64, lot_size: f64, price: f64, timestamp: i64) -> PyResult<()> { + validation::finite("lot_size", lot_size)?; + validation::positive("price", price)?; + self.close_partial(id, lot_size, price, timestamp); + Ok(()) + } + + #[pyo3(name = "update_sl")] + fn py_update_sl(&mut self, id: u64, stop_loss: f64) -> PyResult { + validation::positive("stop_loss", stop_loss)?; + Ok(self.update_sl(id, stop_loss)) + } + + #[pyo3(name = "close_all")] + fn py_close_all(&mut self, price: f64, timestamp: i64) -> PyResult<()> { + validation::positive("price", price)?; + self.close_all(price, timestamp); + Ok(()) + } + + #[pyo3(name = "equity")] + fn py_equity(&self, current_price: f64) -> PyResult { + validation::positive("price", current_price)?; + Ok(self.equity(current_price)) + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/src/engine.rs b/src/engine.rs index 5eda326..eff9930 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -2,6 +2,8 @@ use crate::broker::Broker; use crate::stats::Stats; use crate::strategy::Strategy; use crate::types::Bar; +use crate::validation; +use pyo3::exceptions::PyValueError; use pyo3::prelude::*; #[pyclass] @@ -68,9 +70,7 @@ impl Strategy for SmaCross { } } -#[pymethods] impl Engine { - #[new] pub fn new( data: Vec, initial_cash: f64, @@ -91,6 +91,22 @@ impl Engine { equity_curve: Vec::new(), } } +} + +#[pymethods] +impl Engine { + #[new] + fn py_new( + data: Vec, + initial_cash: f64, + commission: f64, + spread: f64, + contract_size: f64, + quote_to_account: f64, + ) -> PyResult { + validation::config(initial_cash, commission, spread, contract_size, quote_to_account)?; + Ok(Self::new(data, initial_cash, commission, spread, contract_size, quote_to_account)) + } #[pyo3(name = "run")] pub fn run_py(&mut self, py: Python<'_>, strategy: Py) -> PyResult { @@ -135,14 +151,18 @@ impl Engine { } // ponytail: benchmark hook, runs the SMA strategy entirely in Rust via the native run() path. - pub fn run_native_sma(&mut self, fast: usize, slow: usize, lot_size: f64) -> Stats { + pub fn run_native_sma(&mut self, fast: usize, slow: usize, lot_size: f64) -> PyResult { + validation::positive("lot_size", lot_size)?; + if fast == 0 || slow == 0 || fast > slow { + return Err(PyValueError::new_err("SMA windows must satisfy 1 <= fast <= slow")); + } let mut strat = SmaCross { fast, slow, lot_size, closes: Vec::new(), }; - self.run(&mut strat) + Ok(self.run(&mut strat)) } } diff --git a/src/lib.rs b/src/lib.rs index 9c4718a..e93f4b9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -5,6 +5,7 @@ pub mod optimise; pub mod stats; pub mod strategy; pub mod types; +mod validation; use pyo3::prelude::*; @@ -18,5 +19,6 @@ fn backtestingfx(m: &Bound<'_, PyModule>) -> PyResult<()> { m.add_class::()?; m.add_class::()?; m.add_function(wrap_pyfunction!(optimise::run_grid, m)?)?; + m.add_function(wrap_pyfunction!(validation::config, m)?)?; Ok(()) } diff --git a/src/optimise.rs b/src/optimise.rs index 448c404..848d05f 100644 --- a/src/optimise.rs +++ b/src/optimise.rs @@ -83,6 +83,13 @@ pub fn run_grid( bars ))); } + for (signal_index, signal) in signals.iter().enumerate() { + if let Some(bar_index) = signal.iter().position(|value| !value.is_finite()) { + return Err(PyValueError::new_err(format!( + "signal {signal_index} at bar {bar_index} must be finite" + ))); + } + } let engine: &Engine = &engine; let threads = std::thread::available_parallelism().map_or(1, |n| n.get()); diff --git a/src/validation.rs b/src/validation.rs new file mode 100644 index 0000000..e9288ca --- /dev/null +++ b/src/validation.rs @@ -0,0 +1,59 @@ +//! Numeric checks shared by the public Python entry points. + +use pyo3::exceptions::PyValueError; +use pyo3::prelude::*; + +pub(crate) fn finite(name: &str, value: f64) -> PyResult<()> { + if !value.is_finite() { + return Err(PyValueError::new_err(format!("{name} must be finite"))); + } + Ok(()) +} + +pub(crate) fn positive(name: &str, value: f64) -> PyResult<()> { + finite(name, value)?; + if value <= 0.0 { + return Err(PyValueError::new_err(format!("{name} must be greater than zero"))); + } + Ok(()) +} + +fn nonnegative(name: &str, value: f64) -> PyResult<()> { + finite(name, value)?; + if value < 0.0 { + return Err(PyValueError::new_err(format!("{name} must be nonnegative"))); + } + Ok(()) +} + +#[pyfunction(name = "_validate_config")] +pub(crate) fn config( + cash: f64, + commission: f64, + spread: f64, + contract_size: f64, + quote_to_account: f64, +) -> PyResult<()> { + positive("cash", cash)?; + nonnegative("commission", commission)?; + nonnegative("spread", spread)?; + positive("contract_size", contract_size)?; + positive("quote_to_account", quote_to_account) +} + +pub(crate) fn order( + price: f64, + lot_size: f64, + stop_loss: Option, + take_profit: Option, +) -> PyResult<()> { + positive("price", price)?; + positive("lot_size", lot_size)?; + if let Some(stop) = stop_loss { + positive("stop_loss", stop)?; + } + if let Some(target) = take_profit { + positive("take_profit", target)?; + } + Ok(()) +} diff --git a/tests/test_numeric_validation.py b/tests/test_numeric_validation.py new file mode 100644 index 0000000..6a32175 --- /dev/null +++ b/tests/test_numeric_validation.py @@ -0,0 +1,150 @@ +import math +import unittest + +import numpy as np +import pandas as pd + +from backtestingfx import Backtest, Bar, Broker, Engine, Strategy +from backtestingfx import _backtestingfx as native + + +def market(): + return pd.DataFrame( + {column: [1.1, 1.1] for column in ("open", "high", "low", "close")}, + index=pd.date_range("2026-01-01", periods=2, freq="h", tz="UTC"), + ) + + +def broker(): + return Broker(10_000.0, 7.0, 0.0001, 100_000.0, 1.27) + + +def engine(): + return Engine([Bar(i, 1.1, 1.1, 1.1, 1.1, 0.0) for i in range(2)], + 10_000.0, 7.0, 0.0001, 100_000.0, 1.27) + + +class NumericValidationTest(unittest.TestCase): + def test_configuration_rejected_at_all_python_constructors(self): + defaults = [10_000.0, 0.0, 0.0, 100_000.0, 1.0] + names = ["cash", "commission", "spread", "contract_size", "quote_to_account"] + for index, name in enumerate(names): + invalid = [math.nan, math.inf, -math.inf, -1.0] + if name not in ("commission", "spread"): + invalid.append(0.0) + for value in invalid: + values = defaults.copy() + values[index] = value + for label, constructor in ( + ("Broker", lambda: Broker(*values)), + ("Engine", lambda: Engine([], *values)), + ("Backtest", lambda: Backtest(market(), **dict(zip(names, values)))), + ): + with self.subTest(name=name, value=value, constructor=label): + with self.assertRaisesRegex(ValueError, name): + constructor() + + def test_invalid_orders_leave_cash_positions_and_ids_unchanged(self): + for method in ("buy", "sell"): + for name in ("lot_size", "price", "stop_loss", "take_profit"): + for value in (0.0, -1.0, math.nan, math.inf, -math.inf): + with self.subTest(method=method, name=name, value=value): + account = broker() + args = {"price": 1.1, "lot_size": 0.4, "timestamp": 0, + "stop_loss": None, "take_profit": None} + args[name] = value + with self.assertRaisesRegex(ValueError, name): + getattr(account, method)(**args) + self.assertEqual(account.cash, 10_000.0) + self.assertEqual(account.positions(), []) + account.buy(1.1, 0.4, 0, None, None) + self.assertEqual(account.positions()[0].id, 0) + + def test_invalid_partial_closes_and_stop_updates_preserve_position(self): + for value in (math.nan, math.inf, -math.inf): + with self.subTest(value=value): + account = broker() + account.buy(1.1, 1.0, 0, 1.09, None) + cash = account.cash + with self.assertRaisesRegex(ValueError, "lot_size"): + account.close_partial(0, value, 1.1, 1) + with self.assertRaisesRegex(ValueError, "stop_loss"): + account.update_sl(0, value) + self.assertEqual(account.cash, cash) + self.assertEqual(account.positions()[0].lot_size, 1.0) + self.assertEqual(account.positions()[0].stop_loss, 1.09) + + def test_partial_close_noops_and_profit_trailing_stops_still_work(self): + account = broker() + account.buy(1.1, 1.0, 0, 1.09, None) + for lots in (0.0, -1.0): + account.close_partial(0, lots, 1.1, 1) + account.close_partial(999, 0.4, 1.1, 1) + self.assertFalse(account.update_sl(999, 1.09)) + self.assertTrue(account.update_sl(0, 1.105)) + self.assertEqual(account.positions()[0].lot_size, 1.0) + self.assertAlmostEqual(account.cash, 9993.0) + account.close_partial(0, 2.0, 1.1, 1) + self.assertEqual(account.positions(), []) + self.assertAlmostEqual(account.cash, 9973.3) + + def test_invalid_close_and_equity_prices_raise_before_mutation(self): + for value in (0.0, -1.0, math.nan, math.inf, -math.inf): + for method in ("close_all", "close_position", "close_partial", "equity"): + with self.subTest(method=method, value=value): + account = broker() + account.buy(1.1, 1.0, 0, None, None) + args = {"close_all": (value, 1), "close_position": (0, value, 1), + "close_partial": (0, 0.4, value, 1), "equity": (value,)} + with self.assertRaisesRegex(ValueError, "price"): + getattr(account, method)(*args[method]) + self.assertEqual(len(account.positions()), 1) + self.assertAlmostEqual(account.cash, 9993.0) + + def test_strategy_order_errors_propagate_as_value_errors(self): + class InvalidOrder(Strategy): + def next(self): + self.buy(math.nan) + + with self.assertRaisesRegex(ValueError, "lot_size"): + Backtest(market(), InvalidOrder).run() + + def test_optimize_rejects_nonfinite_and_malformed_signals(self): + cases = [([0.0, math.inf], "finite"), ([math.nan, 0.0], "NaN"), + ([0.0, -math.inf], "finite"), ([[0.0], [0.0]], "one-dimensional"), + (0.1, "one-dimensional"), ([0.1], "one per bar")] + for signal, message in cases: + with self.subTest(signal=signal): + with self.assertRaisesRegex(ValueError, message): + Backtest(market()).optimize(lambda df, unused: signal, unused=[1]) + + def test_direct_grid_rejects_nonfinite_signals_in_every_combination(self): + for value in (math.nan, math.inf, -math.inf): + with self.subTest(value=value): + with self.assertRaisesRegex(ValueError, "signal 1.*bar 1.*finite"): + native.run_grid(engine(), [[0.0, 0.0], [0.0, value]]) + with self.assertRaisesRegex(ValueError, "one per bar"): + native.run_grid(engine(), [[0.0]]) + + def test_signed_and_flat_signals_remain_valid(self): + results = Backtest(market()).optimize( + lambda df, lots: np.full(len(df), lots), lots=[-0.4, 0.0, 0.4], + ) + self.assertEqual(len(results), 3) + for params, stats in results: + self.assertAlmostEqual(stats.final_cash, 10_000.0) + self.assertEqual(stats.num_trades, 0 if params["lots"] == 0 else 1) + + def test_native_sma_rejects_invalid_sizes_and_windows_without_panicking(self): + for lots in (0.0, -1.0, math.nan, math.inf, -math.inf): + with self.subTest(lots=lots): + with self.assertRaisesRegex(ValueError, "lot_size"): + engine().run_native_sma(1, 2, lots) + for fast, slow in ((0, 2), (1, 0), (3, 1)): + with self.subTest(fast=fast, slow=slow): + with self.assertRaisesRegex(ValueError, "fast|slow"): + engine().run_native_sma(fast, slow, 0.4) + + +if __name__ == "__main__": + unittest.main()