gitadmin/foxhunt
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(risk): return Option<f64> for insufficient-samples Sharpe/Sortino

Browse Source 
services/trading_service/src/services/risk.rs compute_sharpe_ratio /
compute_sortino_ratio previously returned `0.0` as an
"insufficient-samples" fallback. Plain zero is indistinguishable
from a legitimate "Sharpe happened to equal 0" result; callers
gating on `sharpe > threshold` silently pass/fail on the fallback,
and downstream monitoring logs it as a real value.

Now: returns `Option<f64>` — `Some(value)` for valid samples,
`None` when:
  * returns.len() < MIN_RETURN_OBSERVATIONS (threshold at line 30
    unchanged — only the return-shape changes)
  * daily_std < 1e-12 (degenerate zero-volatility series; ratio
    mathematically undefined, not zero)
  * downside_count < 2 (Sortino only; too few sub-risk-free
    observations to estimate downside variance)
  * downside_dev < 1e-12 (Sortino only)

Production caller (get_risk_metrics, the only call site) updated:
  * On Some/Some: debug-log as before
  * On either None: WARN-log with `[insufficient-samples]` tag,
    formatting each ratio as "None" or "{:.4}" so operators can
    distinguish missing data from a real zero
  * Protobuf RiskMetrics { sharpe_ratio, sortino_ratio } is a
    non-optional `double` in risk.proto — export `f64::NAN` rather
    than a fake 0, so consumers can detect the undefined case
    without silently failing threshold gates.

Tests: each function now has happy-path `Some(_)`,
insufficient-samples `None`, and empty-input `None` cases;
existing `_zero_volatility` / `_no_downside` tests re-asserted
against `None` (previously asserted against 0.0).

compute_volatility and compute_current_drawdown retain `f64`
return — their 0.0 outputs are legitimate (no position → 0%
drawdown; constant returns → 0% volatility).

Callers touched:
  - services/trading_service/src/services/risk.rs:570-596
    (get_risk_metrics — only production caller)
  - services/trading_service/src/services/risk.rs:1585-1671
    (tests)

Cross-file grep (compute_sharpe|compute_sortino|RiskServiceImpl::)
confirms no other callers in crates/, services/, bin/.
position_manager.rs line 772 references the function in a comment
only.

Compile: SQLX_OFFLINE=true CARGO_INCREMENTAL=0 cargo check
--workspace --tests — clean on risk.rs (pre-existing ml-crate
warnings unchanged).
Tests: cargo test -p trading-service --lib risk — 59 passed,
0 failed, including 2 new empty-input tests.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
jgrusewski
2026-04-23 09:24:17 +02:00
parent d9fee6ef8d
commit e906ab96d1
1 changed files with 97 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files

124
services/trading_service/src/services/risk.rs
View File

@@ -27,7 +27,11 @@ pub struct RiskServiceImpl {
const RISK_FREE_RATE_ANNUAL: f64 = 0.05;
/// Minimum number of return observations required for ratio calculations.
/// Below this threshold, Sharpe/Sortino return 0.0 to avoid noisy estimates.
/// Below this threshold, `compute_sharpe_ratio` / `compute_sortino_ratio`
/// return `None` to avoid noisy estimates. Callers MUST distinguish
/// "insufficient samples" from a legitimate zero-valued ratio; returning
/// a plain `0.0` would silently pass/fail threshold gates (e.g.
/// `sharpe > 1.0`) and pollute downstream monitoring with fake zeros.
const MIN_RETURN_OBSERVATIONS: usize = 5;
impl RiskServiceImpl {
@@ -226,10 +230,18 @@ impl RiskServiceImpl {
/// Compute annualized Sharpe ratio from a return series.
///
/// Sharpe = (mean_return - risk_free_daily) / std_dev, annualized.
/// Returns 0.0 if insufficient data or zero volatility.
fn compute_sharpe_ratio(returns: &[f64]) -> f64 {
///
/// Returns `None` in three distinct cases — callers MUST handle each
/// explicitly (conservative gating, `[insufficient-samples]`-tagged
/// logging, NaN metric export):
/// * `returns.len() < MIN_RETURN_OBSERVATIONS` — too few samples.
/// * `returns.len() < 2` — sample variance is undefined.
/// * `daily_std < 1e-12` — the ratio is mathematically undefined
/// (degenerate zero-volatility series), distinct from a legitimate
/// Sharpe that happens to equal 0.
fn compute_sharpe_ratio(returns: &[f64]) -> Option<f64> {
if returns.len() < MIN_RETURN_OBSERVATIONS {
return 0.0;
return None;
}
let n = returns.len() as f64;
@@ -238,24 +250,31 @@ impl RiskServiceImpl {
let daily_std = variance.sqrt();
if daily_std < 1e-12 {
return 0.0;
return None;
}
let daily_rf = RISK_FREE_RATE_ANNUAL / 252.0;
let excess_return = mean_return - daily_rf;
// Annualized Sharpe
(excess_return / daily_std) * 252_f64.sqrt()
Some((excess_return / daily_std) * 252_f64.sqrt())
}
/// Compute annualized Sortino ratio from a return series.
///
/// Sortino = (mean_return - risk_free_daily) / downside_deviation, annualized.
/// Downside deviation only considers returns below the risk-free rate.
/// Returns 0.0 if insufficient data or zero downside deviation.
fn compute_sortino_ratio(returns: &[f64]) -> f64 {
///
/// Returns `None` when the ratio cannot be computed — callers MUST
/// handle this explicitly (do NOT substitute a default 0.0, which would
/// be indistinguishable from a legitimate zero-valued Sortino):
/// * `returns.len() < MIN_RETURN_OBSERVATIONS` — too few samples.
/// * `downside_count < 2` — not enough sub-risk-free returns to
/// estimate downside variance.
/// * `downside_dev < 1e-12` — downside deviation is degenerate.
fn compute_sortino_ratio(returns: &[f64]) -> Option<f64> {
if returns.len() < MIN_RETURN_OBSERVATIONS {
return 0.0;
return None;
}
let n = returns.len() as f64;
@@ -271,18 +290,18 @@ impl RiskServiceImpl {
let downside_count = returns.iter().filter(|&&r| r < daily_rf).count();
if downside_count < 2 {
return 0.0;
return None;
}
let downside_dev = (downside_sq_sum / (downside_count as f64 - 1.0)).sqrt();
if downside_dev < 1e-12 {
return 0.0;
return None;
}
let excess_return = mean_return - daily_rf;
// Annualized Sortino
(excess_return / downside_dev) * 252_f64.sqrt()
Some((excess_return / downside_dev) * 252_f64.sqrt())
}
/// Build per-position risk entries using position data and VaR from the risk engine.
@@ -549,13 +568,32 @@ impl RiskService for RiskServiceImpl {
let num_returns = returns.len();
let volatility = Self::compute_volatility(&returns);
let sharpe_ratio = Self::compute_sharpe_ratio(&returns);
let sortino_ratio = Self::compute_sortino_ratio(&returns);
let sharpe_opt = Self::compute_sharpe_ratio(&returns);
let sortino_opt = Self::compute_sortino_ratio(&returns);
debug!(
"Risk metrics computed: drawdown={:.4}, vol={:.4}, sharpe={:.4}, sortino={:.4}, returns_n={}",
current_drawdown, volatility, sharpe_ratio, sortino_ratio, num_returns
);
// Export NaN when the ratio is undefined (insufficient samples,
// zero volatility, or too few downside observations). NaN is the
// correct wire-format sentinel for a non-optional `double` in the
// protobuf schema — it is distinguishable from a legitimate 0.0
// by consumers and will not silently pass/fail threshold gates.
let sharpe_ratio = sharpe_opt.unwrap_or(f64::NAN);
let sortino_ratio = sortino_opt.unwrap_or(f64::NAN);
match (sharpe_opt, sortino_opt) {
(Some(s), Some(so)) => debug!(
"Risk metrics computed: drawdown={:.4}, vol={:.4}, sharpe={:.4}, sortino={:.4}, returns_n={}",
current_drawdown, volatility, s, so, num_returns
),
_ => warn!(
"Risk metrics computed [insufficient-samples]: drawdown={:.4}, vol={:.4}, sharpe={}, sortino={}, returns_n={} (min required={})",
current_drawdown,
volatility,
sharpe_opt.map_or_else(|| "None".to_string(), |v| format!("{:.4}", v)),
sortino_opt.map_or_else(|| "None".to_string(), |v| format!("{:.4}", v)),
num_returns,
MIN_RETURN_OBSERVATIONS
),
}
// Build per-position risk entries with VaR contributions
let position_risks = self.build_position_risks(&positions).await;
@@ -1544,19 +1582,33 @@ mod tests {
// -----------------------------------------------------------------------
// 6. compute_sharpe_ratio
// -----------------------------------------------------------------------
#[test]
fn test_sharpe_empty_returns() {
// Empty slice: below MIN_RETURN_OBSERVATIONS => None
let sharpe = RiskServiceImpl::compute_sharpe_ratio(&[]);
assert!(sharpe.is_none(), "Empty returns must yield None, not fake 0.0");
}
#[test]
fn test_sharpe_insufficient_data() {
// Fewer than MIN_RETURN_OBSERVATIONS (5) returns
let returns = vec![0.01, 0.02, 0.01, -0.01];
let sharpe = RiskServiceImpl::compute_sharpe_ratio(&returns);
assert!((sharpe - 0.0).abs() < 1e-10, "Should return 0.0 for insufficient data");
assert!(
sharpe.is_none(),
"Should return None for insufficient data (not fake 0.0 — caller must distinguish \
insufficient-samples from a legitimate zero-valued Sharpe)"
);
}
#[test]
fn test_sharpe_zero_volatility() {
// Degenerate zero-volatility series: ratio is mathematically undefined,
// MUST return None (not 0.0 — that would be indistinguishable from a
// real Sharpe ratio that happens to equal 0).
let returns = vec![0.01; 10];
let sharpe = RiskServiceImpl::compute_sharpe_ratio(&returns);
assert!((sharpe - 0.0).abs() < 1e-10, "Zero volatility should return 0.0 Sharpe");
assert!(sharpe.is_none(), "Zero-volatility series must yield None, not fake 0.0");
}
#[test]
@@ -1564,28 +1616,45 @@ mod tests {
// Use enough data points with positive mean return
let returns = vec![0.01, 0.02, 0.015, 0.005, 0.012, 0.008];
let sharpe = RiskServiceImpl::compute_sharpe_ratio(&returns);
// Mean return is clearly positive and above risk-free => Sharpe should be positive
assert!(sharpe > 0.0, "Sharpe should be positive for consistently positive returns; got {}", sharpe);
// Mean return is clearly positive and above risk-free => Sharpe should be Some(positive)
let value = sharpe.expect("Sharpe should be Some(_) for a well-conditioned return series");
assert!(value > 0.0, "Sharpe should be positive for consistently positive returns; got {}", value);
}
// -----------------------------------------------------------------------
// 7. compute_sortino_ratio
// -----------------------------------------------------------------------
#[test]
fn test_sortino_empty_returns() {
// Empty slice: below MIN_RETURN_OBSERVATIONS => None
let sortino = RiskServiceImpl::compute_sortino_ratio(&[]);
assert!(sortino.is_none(), "Empty returns must yield None, not fake 0.0");
}
#[test]
fn test_sortino_insufficient_data() {
let returns = vec![0.01, -0.01, 0.02];
let sortino = RiskServiceImpl::compute_sortino_ratio(&returns);
assert!((sortino - 0.0).abs() < 1e-10);
assert!(
sortino.is_none(),
"Should return None for insufficient data (not fake 0.0 — caller must distinguish \
insufficient-samples from a legitimate zero-valued Sortino)"
);
}
#[test]
fn test_sortino_no_downside() {
// All returns well above risk-free => no downside deviation => 0
// All returns well above risk-free => zero downside observations =>
// downside deviation is undefined, MUST return None (not fake 0.0).
let daily_rf = RISK_FREE_RATE_ANNUAL / 252.0;
let high_return = daily_rf + 0.01; // well above risk-free
let returns = vec![high_return; 10];
let sortino = RiskServiceImpl::compute_sortino_ratio(&returns);
assert!((sortino - 0.0).abs() < 1e-10, "No downside should yield zero Sortino");
assert!(
sortino.is_none(),
"No downside observations must yield None (downside deviation is undefined), \
not fake 0.0"
);
}
#[test]
@@ -1593,8 +1662,9 @@ mod tests {
// Mix of positive and negative returns
let returns = vec![-0.02, 0.03, -0.01, 0.02, -0.015, 0.01, 0.005];
let sortino = RiskServiceImpl::compute_sortino_ratio(&returns);
// Just verify it produces a finite number (not NaN/Inf)
assert!(sortino.is_finite(), "Sortino should be finite; got {}", sortino);
// Just verify it produces Some(finite) (not None/NaN/Inf)
let value = sortino.expect("Sortino should be Some(_) with mixed returns");
assert!(value.is_finite(), "Sortino should be finite; got {}", value);
}
// -----------------------------------------------------------------------