Skip to content

Commit 6fd7294

Browse files
committed
address review: finish checked-d_* in volatility kernels
Addresses three inline review comments on #371 about silent saturation paths remaining in the volatility statistics helpers. - `constant_volatility`: * centred deviations now use `d_sub(r, mean, ..)`; * the square is built via `d_mul(centred, centred, ..)` instead of the unchecked `.powi(2)` which silently saturates on overflow; * the sum of squares is folded inline with `d_add` so the intermediate `Vec<Decimal>` allocation is gone. - `ewma_volatility`: * the initial variance seed `first_return^2` is built via `d_mul` for the same reason; * the per-step `r^2` innovation is likewise pre-built with `d_mul` and then folded into the checked `(1 - λ) · r² + λ · v_prev` recursion. - `garch_volatility`: * the initial variance seed `r_0^2` is built via `d_mul`; * the per-step `r^2` is likewise built via `d_mul` before entering `α · r²`; * the silent `Positive::new_decimal(..).unwrap_or(Positive::ZERO)` clamp on negative variance is replaced with a helper closure that returns `VolatilityError::NumericalFailure { reason: .. }`, so pathological GARCH parameters now surface a readable diagnostic with the offending variance value instead of being hidden behind a zero-vol result; * the `# Errors` section of `garch_volatility` is rewritten to describe both the new `DecimalError::Overflow` propagation path and the `NumericalFailure` path, replacing the previous "currently infallible" note. No public signatures change. `cargo test --lib` passes (3751 / 5 ignored / 0 regressions). `cargo clippy --lib --all-features` reports zero warnings. Refs #335, PR #371
1 parent e292bee commit 6fd7294

1 file changed

Lines changed: 72 additions & 16 deletions

File tree

src/volatility/utils.rs

Lines changed: 72 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -51,13 +51,22 @@ pub fn constant_volatility(returns: &[Decimal]) -> Result<Positive, VolatilityEr
5151
return Ok(Positive::ZERO);
5252
}
5353

54-
// Sample mean and sample variance. The sums are unbounded so checked_*
55-
// is applied via `d_sum` (for the returns sum) and `d_div` (for the
56-
// mean and variance projection onto bankers-rounded Decimal).
54+
// Sample mean and sample variance. Every step is checked:
55+
// * the raw sum is folded with `d_sum`;
56+
// * the mean projection uses `d_div` with banker's rounding;
57+
// * each centred deviation is built with `d_sub`;
58+
// * the square is built with `d_mul` (no `.powi(2)`, which is
59+
// unchecked and can saturate on adversarial inputs);
60+
// * the sum of squares is folded inline so we do not allocate a
61+
// temporary `Vec`.
5762
let total = d_sum(returns, "volatility::constant::total")?;
5863
let mean = d_div(total, n.to_dec(), "volatility::constant::mean")?;
59-
let centred_sq: Vec<Decimal> = returns.iter().map(|&r| (r - mean).powi(2)).collect();
60-
let sq_total = d_sum(&centred_sq, "volatility::constant::sq_total")?;
64+
let mut sq_total = Decimal::ZERO;
65+
for &r in returns {
66+
let centred = d_sub(r, mean, "volatility::constant::centred")?;
67+
let centred_sq = d_mul(centred, centred, "volatility::constant::centred_sq")?;
68+
sq_total = d_add(sq_total, centred_sq, "volatility::constant::sq_total")?;
69+
}
6170
let variance = d_div(
6271
sq_total,
6372
(n - Decimal::ONE).to_dec(),
@@ -135,7 +144,13 @@ pub fn ewma_volatility(
135144
.ok_or_else(|| VolatilityError::NumericalFailure {
136145
reason: "ewma_volatility: returns slice is empty".to_string(),
137146
})?;
138-
let mut variance = first_return.powi(2);
147+
// `first_return^2` is built with a checked multiplication because
148+
// `.powi(2)` on `Decimal` silently saturates on overflow.
149+
let mut variance = d_mul(
150+
*first_return,
151+
*first_return,
152+
"volatility::ewma::initial_variance",
153+
)?;
139154
let initial_std_dev = variance
140155
.sqrt()
141156
.ok_or_else(|| VolatilityError::NumericalFailure {
@@ -145,11 +160,19 @@ pub fn ewma_volatility(
145160

146161
for &return_value in &returns[1..] {
147162
// EWMA variance recursion: v' = λ·v + (1 - λ) · r².
163+
// `r²` is built via `d_mul(r, r, ..)` so that a saturating
164+
// squaring cannot feed a silently capped value into the
165+
// innovation term.
148166
let persistent = d_mul(lambda, variance, "volatility::ewma::persistent")?;
149167
let innovation_weight = d_sub(Decimal::ONE, lambda, "volatility::ewma::innovation_weight")?;
168+
let return_sq = d_mul(
169+
return_value,
170+
return_value,
171+
"volatility::ewma::return_sq",
172+
)?;
150173
let innovation = d_mul(
151174
innovation_weight,
152-
return_value.powi(2),
175+
return_sq,
153176
"volatility::ewma::innovation",
154177
)?;
155178
variance = d_add(persistent, innovation, "volatility::ewma::variance")?;
@@ -303,12 +326,18 @@ pub fn calculate_iv(
303326
///
304327
/// # Errors
305328
///
306-
/// Currently infallible — every `Positive::new_decimal(...)` call
307-
/// is clamped to `Positive::ZERO`, so neither `NumericalFailure` nor
308-
/// `PositiveError` is surfaced. The `Result` signature is retained
309-
/// so future implementations that add explicit stationarity checks
310-
/// (`alpha + beta < 1`) or overflow validation can return
311-
/// `VolatilityError::NumericalFailure` without a breaking change.
329+
/// - Propagates [`DecimalError::Overflow`] from the underlying
330+
/// checked arithmetic helpers (`d_mul`, `d_add`) wrapped as
331+
/// [`VolatilityError::DecimalError`] via the `#[from]` cascade.
332+
/// This happens when any of the four monetary products
333+
/// (`r^2`, `α · r²`, `β · v_prev`, `ω + α · r² + β · v_prev`)
334+
/// exceeds the representable `Decimal` range, or when the
335+
/// squared initial return itself overflows.
336+
/// - Returns [`VolatilityError::NumericalFailure`] when a recursion
337+
/// step produces a negative variance (e.g. pathological GARCH
338+
/// parameters with `omega` or `beta * v_prev` negative). This is
339+
/// a real diagnostic now instead of being silently clamped to
340+
/// `Positive::ZERO` as in earlier implementations.
312341
///
313342
/// # Panics
314343
///
@@ -320,16 +349,43 @@ pub fn garch_volatility(
320349
alpha: Decimal,
321350
beta: Decimal,
322351
) -> Result<Vec<Positive>, VolatilityError> {
323-
let mut variance = Positive::new_decimal(returns[0].powi(2)).unwrap_or(Positive::ZERO);
352+
// Helper: project a variance `Decimal` onto `Positive`, mapping
353+
// the rejection (negative variance) onto a readable
354+
// `VolatilityError::NumericalFailure` instead of silently
355+
// clamping to zero.
356+
let to_positive_variance = |value: Decimal,
357+
context: &'static str|
358+
-> Result<Positive, VolatilityError> {
359+
Positive::new_decimal(value).map_err(|_| VolatilityError::NumericalFailure {
360+
reason: format!(
361+
"garch_volatility: negative variance at {context} ({value})"
362+
),
363+
})
364+
};
365+
366+
// Seed the recursion with `r_0^2` via `d_mul` so a saturating
367+
// squaring cannot feed a silently capped value into the GARCH
368+
// update below.
369+
let seed = d_mul(
370+
returns[0],
371+
returns[0],
372+
"volatility::garch::initial_variance",
373+
)?;
374+
let mut variance = to_positive_variance(seed, "initial_variance")?;
324375
let mut volatilities = vec![variance.sqrt()];
325376
for &return_value in &returns[1..] {
326377
// GARCH(1, 1) variance recursion:
327378
// v' = ω + α · r² + β · v_prev.
328-
let alpha_r2 = d_mul(alpha, return_value.powi(2), "volatility::garch::alpha_r2")?;
379+
let return_sq = d_mul(
380+
return_value,
381+
return_value,
382+
"volatility::garch::return_sq",
383+
)?;
384+
let alpha_r2 = d_mul(alpha, return_sq, "volatility::garch::alpha_r2")?;
329385
let beta_v = d_mul(beta, variance.to_dec(), "volatility::garch::beta_v")?;
330386
let persistent = d_add(omega, alpha_r2, "volatility::garch::omega_plus_alpha")?;
331387
let next_variance = d_add(persistent, beta_v, "volatility::garch::variance")?;
332-
variance = Positive::new_decimal(next_variance).unwrap_or(Positive::ZERO);
388+
variance = to_positive_variance(next_variance, "variance")?;
333389
volatilities.push(variance.sqrt());
334390
}
335391
Ok(volatilities)

0 commit comments

Comments
 (0)