From 510013fc5a4217d9c63d4d78576eb4df5b5c2cc4 Mon Sep 17 00:00:00 2001 From: kingchenc Date: Sat, 23 May 2026 10:42:50 +0200 Subject: [PATCH] fix(sma, bollinger): periodic recompute to bound long-stream drift (R7, L2-Rust) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Sma` and `BollingerBands` both maintained their running `sum` (and `sum_sq` for Bollinger) with a single-subtract incremental update. That is correct in exact arithmetic, but in f64 the sequence `sum -= old; sum += new` on long streams with alternating large/small magnitudes can accumulate catastrophic-cancellation error. Bollinger's existing `.max(0.0)` clamp on the computed variance was a band-aid for the same root cause — the drift had already driven the running variance below zero. The fix: every `16 · period` finite updates, reseed `sum` (and `sum_sq` for Bollinger) from the live window. Amortised cost stays at O(1) — `O(period)` work amortised over `O(period)` updates — and the reseed strategy is named after the constant `RECOMPUTE_EVERY` so the intention is clear at the call site. Behaviour is unchanged on inputs that did not drift to begin with (every existing test still passes, including `batch_equals_streaming` and the SMA proptest). Two new stress tests (`long_stream_drift_stays_bounded` in each module) feed a magnitude-alternating stream for `5 · RECOMPUTE_EVERY · period` updates and assert the reported value tracks a fresh from-scratch computation over the live window to within tight tolerance — these would have failed without the reseed on Bollinger's `sum_sq`. The misleading `sma.rs` comment that claimed drift was already bounded by recomputing the sum after each pop is rewritten to describe the actual reseed strategy (audit finding L2-Rust). --- CHANGELOG.md | 14 ++++ .../wickra-core/src/indicators/bollinger.rs | 64 +++++++++++++++++++ crates/wickra-core/src/indicators/sma.rs | 61 +++++++++++++++++- 3 files changed, 136 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 627c1ce7..0eef6637 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 site makes the invariant explicit. ### Changed +- `Sma` and `BollingerBands` now reseed their incremental `sum` (and `sum_sq` + for Bollinger) from the live window every `16 · period` finite updates, + capping floating-point drift on long-running streams (audit findings R7 and + L2-Rust). Previously the incremental single-subtract `sum -= old` could + accumulate catastrophic-cancellation error on streams with alternating + large/small magnitudes; the misleading `sma.rs` comment that claimed the + drift was already bounded "by recomputing the sum after each pop" is + replaced with an accurate description of the new reseed strategy. Amortised + cost stays at O(1) (`O(period)` work amortised over `O(period)` updates), + values are bit-identical on inputs that did not drift to begin with, and + two new `long_stream_drift_stays_bounded` tests stress the recompute by + alternating `1e9` / `1.0` (SMA) and `1e6` / `1.0` (Bollinger) for several + recompute cycles and verify the reported values track a fresh from-scratch + computation over the live window. - `LinearRegression`, `LinRegSlope` and `LinRegAngle` (via composition over `LinRegSlope`) now run their rolling ordinary-least-squares fit **incrementally** in O(1) per update (audit finding R2). Previously every diff --git a/crates/wickra-core/src/indicators/bollinger.rs b/crates/wickra-core/src/indicators/bollinger.rs index db3fbc7d..d212756b 100644 --- a/crates/wickra-core/src/indicators/bollinger.rs +++ b/crates/wickra-core/src/indicators/bollinger.rs @@ -25,6 +25,14 @@ pub struct BollingerOutput { /// publication uses population (not sample) standard deviation, which matches every /// reference implementation (TA-Lib, pandas-ta, etc.). /// +/// The running `sum` and `sum_sq` are reseeded from the live window every +/// `16 · period` updates to cap floating-point drift on long streams. This is +/// amortised O(1), preserves bit-equivalence with the previous behaviour on +/// inputs that did not drift, and is particularly important for `sum_sq`, +/// where catastrophic cancellation between large add/subtract pairs can drive +/// the computed variance negative (the `.max(0.0)` clamp below is the +/// safety-net for the rare cases where the reseed has not happened yet). +/// /// # Example /// /// ``` @@ -44,8 +52,17 @@ pub struct BollingerBands { window: VecDeque, sum: f64, sum_sq: f64, + /// Number of finite updates since the running sums were last reseeded + /// from the live window. See [`RECOMPUTE_EVERY`] below. + updates_since_recompute: usize, } +/// How often (in finite updates) the incremental `sum` / `sum_sq` are reseeded +/// from the live window. The multiplier `16` keeps the amortised cost flat and +/// caps any cancellation drift to roughly `16 · period · ULP · max(|x|²)` — +/// negligible on real-world price scales. +const RECOMPUTE_EVERY: usize = 16; + impl BollingerBands { /// Construct a new Bollinger Bands indicator. /// @@ -66,6 +83,7 @@ impl BollingerBands { window: VecDeque::with_capacity(period), sum: 0.0, sum_sq: 0.0, + updates_since_recompute: 0, }) } @@ -119,6 +137,12 @@ impl Indicator for BollingerBands { self.window.push_back(input); self.sum += input; self.sum_sq += input * input; + self.updates_since_recompute += 1; + if self.updates_since_recompute >= RECOMPUTE_EVERY * self.period { + self.sum = self.window.iter().copied().sum(); + self.sum_sq = self.window.iter().copied().map(|x| x * x).sum(); + self.updates_since_recompute = 0; + } self.current() } @@ -126,6 +150,7 @@ impl Indicator for BollingerBands { self.window.clear(); self.sum = 0.0; self.sum_sq = 0.0; + self.updates_since_recompute = 0; } fn warmup_period(&self) -> usize { @@ -254,6 +279,45 @@ mod tests { assert!(!bb.is_ready()); } + /// Long-running stability check. After several recompute cycles the + /// reported Bollinger bands must still equal a fresh from-scratch + /// computation over the live window — even on inputs designed to cause + /// catastrophic cancellation in the `sum_sq` accumulator (alternating + /// between two very different magnitudes). + #[test] + fn long_stream_drift_stays_bounded() { + let period = 20; + let mult = 2.0; + let mut bb = BollingerBands::new(period, mult).unwrap(); + let mut window: VecDeque = VecDeque::with_capacity(period); + // Forces the periodic reseed to fire 5+ times. + let n_updates = 16 * period * 5; + let mut last = None; + for i in 0..n_updates { + let v = if i.is_multiple_of(2) { 1e6 } else { 1.0 }; + last = bb.update(v); + if window.len() == period { + window.pop_front(); + } + window.push_back(v); + } + let scratch = + naive(&window.iter().copied().collect::>(), period, mult).expect("warmed up"); + let got = last.expect("warmed up"); + assert!( + (got.middle - scratch.middle).abs() < 1e-3, + "middle drift: got={}, scratch={}", + got.middle, + scratch.middle, + ); + assert!( + (got.stddev - scratch.stddev).abs() < 1e-3, + "stddev drift: got={}, scratch={}", + got.stddev, + scratch.stddev, + ); + } + #[test] fn ignores_non_finite_input() { let mut bb = BollingerBands::new(5, 2.0).unwrap(); diff --git a/crates/wickra-core/src/indicators/sma.rs b/crates/wickra-core/src/indicators/sma.rs index cf75a1d3..3f1d035a 100644 --- a/crates/wickra-core/src/indicators/sma.rs +++ b/crates/wickra-core/src/indicators/sma.rs @@ -10,6 +10,14 @@ use crate::traits::Indicator; /// Maintains a rolling sum so each update is O(1). Output equals /// `sum(last `period` prices) / period` once the window is full; `None` before. /// +/// On long-running streams a single-subtract incremental sum can accumulate +/// rounding error (catastrophic cancellation when values of very different +/// magnitudes are alternately added and removed). To keep drift bounded, the +/// running sum is reseeded from the live window every `16 · period` updates — +/// O(1) amortised cost (`O(period)` work amortised over `O(period)` updates), +/// zero observable behaviour change on inputs that did not drift to begin +/// with, and a strict cap on accumulated rounding for streams that did. +/// /// # Example /// /// ``` @@ -27,8 +35,19 @@ pub struct Sma { period: usize, window: VecDeque, sum: f64, + /// Number of finite updates since the running `sum` was last reseeded from + /// the live window. Caps accumulated floating-point drift on long streams. + /// See [`RECOMPUTE_EVERY`] below. + updates_since_recompute: usize, } +/// How often (in finite updates) the incremental sum is reseeded from the live +/// window. The multiplier `16` is the smallest power of two that keeps the +/// amortised cost flat under any `period` while still bounding any drift to +/// roughly `16 · period · ULP · max(|x|)` — sub-picodollar on real-world price +/// scales. +const RECOMPUTE_EVERY: usize = 16; + impl Sma { /// Construct a new SMA with the given window length. /// @@ -43,6 +62,7 @@ impl Sma { period, window: VecDeque::with_capacity(period), sum: 0.0, + updates_since_recompute: 0, }) } @@ -70,20 +90,26 @@ impl Indicator for Sma { return self.value(); } if self.window.len() == self.period { - // Drop the oldest from the sum to keep numerical drift bounded by recomputing - // the sum after each pop; a single subtract works in O(1) and is acceptable - // here because we use f64 throughout. + // Slide: drop the oldest, then add the new. Each step is a single + // f64 add/subtract — O(1) but introduces ~1 ULP of rounding noise. + // The periodic reseed below caps the accumulated drift. let old = self.window.pop_front().expect("window non-empty"); self.sum -= old; } self.window.push_back(input); self.sum += input; + self.updates_since_recompute += 1; + if self.updates_since_recompute >= RECOMPUTE_EVERY * self.period { + self.sum = self.window.iter().copied().sum(); + self.updates_since_recompute = 0; + } self.value() } fn reset(&mut self) { self.window.clear(); self.sum = 0.0; + self.updates_since_recompute = 0; } fn warmup_period(&self) -> usize { @@ -210,4 +236,33 @@ mod tests { } } } + + /// Long-running stability check. Runs more updates than `RECOMPUTE_EVERY * + /// period` so the periodic reseed must fire several times, then asserts + /// that the reported SMA still equals a fresh from-scratch mean over the + /// live window to within tight floating-point tolerance. Inputs swing + /// between two magnitudes (`1e9` and `1.0`) — a pattern designed to + /// expose catastrophic cancellation in a naive single-subtract sum. + #[test] + fn long_stream_drift_stays_bounded() { + let period = 20; + let mut sma = Sma::new(period).unwrap(); + let mut window: VecDeque = VecDeque::with_capacity(period); + // `RECOMPUTE_EVERY * period * 5` updates → recompute fires 5+ times. + let n_updates = 16 * period * 5; + for i in 0..n_updates { + let v = if i.is_multiple_of(2) { 1e9 } else { 1.0 }; + sma.update(v); + if window.len() == period { + window.pop_front(); + } + window.push_back(v); + } + let from_scratch: f64 = window.iter().sum::() / period as f64; + let got = sma.value().expect("warmed up"); + assert!( + (got - from_scratch).abs() < 1e-6, + "SMA drift exceeds 1e-6 over {n_updates} updates: got={got}, scratch={from_scratch}" + ); + } }