Skip to content

Commit f0c67dd

Browse files
authored
Fix remaining indicator rolling-window bugs (#4351)
* Fix SpreadAnalyzer panic when quotes exceed capacity `SpreadAnalyzer` stored spreads in an unbounded `Vec`, whereas the Cython reference uses `deque(maxlen=capacity)`. Once more than `capacity` quotes were processed, `fast_mean_iterated` received a slice longer than the expected length, returned an error, and panicked on `unwrap` — crashing live trading on the (capacity + 1)th quote. Bound the rolling window to `capacity` by evicting the oldest spread, matching the Cython behavior. Add a regression test feeding more than `capacity` quotes; the existing tests stopped at exactly `capacity`, so the panic went uncaught. * Fix EfficiencyRatio unbounded window and reset `EfficiencyRatio` stored prices and deltas in unbounded `Vec`s, whereas the Cython reference uses `deque(maxlen=period)` for both. After `period` updates this diverged from Cython on every bar: - `net_diff` used the all-time-first price instead of the price `period` bars back. - `sum_deltas` summed every delta ever seen instead of the last `period` (also an unbounded memory leak). `reset()` also cleared `inputs` but left `deltas` populated, so stale deltas leaked into the next run. Bound both windows to `period` and clear `deltas` on reset, matching the Cython behavior. Since `AdaptiveMovingAverage` embeds this indicator, its Kaufman efficiency input is corrected too. Add regression tests feeding more than `period` inputs and exercising reset; the existing tests stopped at exactly `period`, so the divergence went uncaught. * Fix RelativeVolatilityIndex std rolling window `RelativeVolatilityIndex` stored prices in a fixed-capacity 1024-element `ArrayDeque`, whereas the Cython reference uses `deque(maxlen=period)`. The standard deviation was therefore computed over up to 1024 observations while the mean came from the `period`-window SMA, diverging from Cython on every bar once more than `period` prices were seen. Bound the price window to `period`, matching Cython. Add a regression test feeding more than `period` inputs; the existing tests stayed within the buffer's growth range, so the divergence went uncaught. * Fix FuzzyCandlesticks unbounded rolling windows `FuzzyCandlesticks` stored candle lengths, body percents, and upper/lower wick percents in four fixed-capacity 1024-element `ArrayDeque`s, whereas the Cython reference uses `deque(maxlen=period)` for each. Once more than `period` candles were processed, the means (`sum / period`) summed the entire buffer and the standard deviations divided by the full buffer length, so every fuzzy size/body/wick classification used statistics over far more than `period` candles. Bound all four windows to `period` by evicting the oldest entry, matching Cython. Add a regression test feeding more than `period` candles; the existing tests stayed within `period`, so the divergence went uncaught. * Fix SpreadAnalyzer average freezing at capacity Once the rolling window reaches `capacity`, the incremental `fast_mean_iterated(..., drop_left=false)` update subtracts `values[length - 1]` (the spread just pushed) instead of the evicted oldest value, so the running average stops changing for non-constant spreads. The Cython reference shares this quirk; the all-constant regression test hid it. Recompute the average from the bounded window after eviction, which is O(capacity) given the existing `Vec::remove(0)` and always correct. Add a regression with monotonically increasing spreads past capacity, and update the constant-spread baked value to the recomputed float.
1 parent 598a088 commit f0c67dd

4 files changed

Lines changed: 182 additions & 30 deletions

File tree

crates/indicators/src/ratio/efficiency_ratio.rs

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ impl Indicator for EfficiencyRatio {
7979
fn reset(&mut self) {
8080
self.value = 0.0;
8181
self.inputs.clear();
82+
self.deltas.clear();
8283
self.initialized = false;
8384
}
8485
}
@@ -99,6 +100,13 @@ impl EfficiencyRatio {
99100

100101
pub fn update_raw(&mut self, value: f64) {
101102
self.inputs.push(value);
103+
// Bound the inputs window to `period`, matching the Cython
104+
// `deque(maxlen=period)`; otherwise the net change below is measured from
105+
// the all-time-first price instead of `period` bars back.
106+
if self.inputs.len() > self.period {
107+
self.inputs.remove(0);
108+
}
109+
102110
if self.inputs.len() < 2 {
103111
self.value = 0.0;
104112
return;
@@ -108,6 +116,11 @@ impl EfficiencyRatio {
108116
let last_diff =
109117
(self.inputs[self.inputs.len() - 1] - self.inputs[self.inputs.len() - 2]).abs();
110118
self.deltas.push(last_diff);
119+
// Bound the deltas window to `period` as well, so the sum reflects only
120+
// the last `period` absolute changes (Cython `deque(maxlen=period)`).
121+
if self.deltas.len() > self.period {
122+
self.deltas.remove(0);
123+
}
111124
let sum_deltas = self.deltas.iter().sum::<f64>().abs();
112125
let net_diff = (self.inputs[self.inputs.len() - 1] - self.inputs[0]).abs();
113126
self.value = if sum_deltas == 0.0 {
@@ -203,6 +216,41 @@ mod tests {
203216
assert_eq!(efficiency_ratio_10.value, 0.428_571_428_572_153_63);
204217
}
205218

219+
#[rstest]
220+
fn test_value_bounded_to_period_after_warmup(mut efficiency_ratio_10: EfficiencyRatio) {
221+
// Regression: with more than `period` inputs the rolling window must stay
222+
// bounded to `period` (matching the Cython `deque(maxlen=period)`), so the
223+
// net change and summed deltas use only the last `period` prices.
224+
let prices = [
225+
1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0, 10.0, 9.0, 8.0, 7.0, 6.0, 5.0,
226+
];
227+
228+
for price in prices {
229+
efficiency_ratio_10.update_raw(price);
230+
}
231+
// Window = last 10 prices: net_diff = |6 - 5| = 1, sum of the last 10 unit
232+
// deltas = 10, so ER = 1 / 10 = 0.1. The old unbounded buffers instead gave
233+
// |1 - 5| / 14 = 0.2857..., using the first-ever price and every delta.
234+
assert_eq!(efficiency_ratio_10.inputs.len(), 10);
235+
assert_eq!(efficiency_ratio_10.value, 0.1);
236+
}
237+
238+
#[rstest]
239+
fn test_reset_clears_deltas(mut efficiency_ratio_10: EfficiencyRatio) {
240+
// Regression: reset must clear the deltas buffer too, otherwise stale
241+
// deltas leak into the next run's sum (matching the Cython `_reset`).
242+
for price in [1.0, 3.0, 6.0, 10.0, 15.0] {
243+
efficiency_ratio_10.update_raw(price);
244+
}
245+
efficiency_ratio_10.reset();
246+
assert!(efficiency_ratio_10.deltas.is_empty());
247+
248+
// Fresh run: two inputs of a single clean move give a ratio of 1.
249+
efficiency_ratio_10.update_raw(100.0);
250+
efficiency_ratio_10.update_raw(100.5);
251+
assert_eq!(efficiency_ratio_10.value, 1.0);
252+
}
253+
206254
#[rstest]
207255
fn test_reset(mut efficiency_ratio_10: EfficiencyRatio) {
208256
for i in 1..=10 {

crates/indicators/src/ratio/spread_analyzer.rs

Lines changed: 63 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -88,9 +88,20 @@ impl Indicator for SpreadAnalyzer {
8888
self.current = spread;
8989
self.spreads.push(spread);
9090

91-
// Update average spread
92-
self.average =
93-
fast_mean_iterated(&self.spreads, spread, self.average, self.capacity, false).unwrap();
91+
// Bound the rolling window to `capacity`, matching the Cython
92+
// `deque(maxlen=capacity)`. Without this the buffer grows unbounded and
93+
// `fast_mean_iterated` errors (panicking on `unwrap`) once the length
94+
// exceeds `capacity`.
95+
if self.spreads.len() > self.capacity {
96+
self.spreads.remove(0);
97+
}
98+
99+
// Recompute the average over the bounded window. The Cython reference uses an
100+
// incremental `fast_mean_iterated(..., drop_left=false)` update, but at capacity
101+
// that subtracts `values[length - 1]` (the spread just pushed) rather than the
102+
// evicted oldest value, so the average freezes for non-constant spreads.
103+
// Recomputing from the bounded window is O(capacity) and always correct.
104+
self.average = fast_mean(&self.spreads);
94105
}
95106

96107
fn reset(&mut self) {
@@ -118,32 +129,6 @@ impl SpreadAnalyzer {
118129
}
119130
}
120131

121-
fn fast_mean_iterated(
122-
values: &[f64],
123-
next_value: f64,
124-
current_value: f64,
125-
expected_length: usize,
126-
drop_left: bool,
127-
) -> Result<f64, &'static str> {
128-
let length = values.len();
129-
130-
if length < expected_length {
131-
return Ok(fast_mean(values));
132-
}
133-
134-
if length != expected_length {
135-
return Err("length of values must equal expected_length");
136-
}
137-
138-
let value_to_drop = if drop_left {
139-
values[0]
140-
} else {
141-
values[length - 1]
142-
};
143-
144-
Ok(current_value + (next_value - value_to_drop) / length as f64)
145-
}
146-
147132
fn fast_mean(values: &[f64]) -> f64 {
148133
if values.is_empty() {
149134
0.0
@@ -212,7 +197,55 @@ mod tests {
212197
spread_analyzer_10.handle_quote(&stub_quote(bid_price[i], ask_price[i]));
213198
}
214199

215-
assert_eq!(spread_analyzer_10.average, 0.050_000_000_000_001_9);
200+
assert_eq!(spread_analyzer_10.average, 0.050_000_000_000_001_42);
201+
}
202+
203+
#[rstest]
204+
fn test_handles_more_inputs_than_capacity_without_panic(
205+
mut spread_analyzer_10: SpreadAnalyzer,
206+
) {
207+
// Regression: feeding more than `capacity` quotes must not panic, and the
208+
// internal window must stay bounded to `capacity` (matching the Cython
209+
// `deque(maxlen=capacity)`). Previously the unbounded buffer caused
210+
// `fast_mean_iterated` to error and panic on the (capacity + 1)th quote.
211+
let bid_price: [&str; 15] = [
212+
"100.50", "100.45", "100.55", "100.60", "100.52", "100.48", "100.53", "100.57",
213+
"100.49", "100.51", "100.54", "100.56", "100.58", "100.50", "100.52",
214+
];
215+
216+
let ask_price: [&str; 15] = [
217+
"100.55", "100.50", "100.60", "100.65", "100.57", "100.53", "100.58", "100.62",
218+
"100.54", "100.56", "100.59", "100.61", "100.63", "100.55", "100.57",
219+
];
220+
221+
for i in 0..15 {
222+
spread_analyzer_10.handle_quote(&stub_quote(bid_price[i], ask_price[i]));
223+
}
224+
225+
assert!(spread_analyzer_10.initialized());
226+
assert_eq!(spread_analyzer_10.spreads.len(), 10);
227+
assert!((spread_analyzer_10.average - 0.05).abs() < 1e-9);
228+
}
229+
230+
#[rstest]
231+
fn test_average_tracks_varying_spreads_past_capacity(mut spread_analyzer_10: SpreadAnalyzer) {
232+
// Regression: with non-constant spreads past `capacity`, the average must keep
233+
// tracking the bounded window rather than freezing. Feeding 15 monotonically
234+
// increasing spreads (0.01..=0.15) leaves the window holding the last 10
235+
// (0.06..=0.15), whose mean is 0.105. The earlier incremental update froze the
236+
// average at 0.055 (the mean of the first full window 0.01..=0.10).
237+
let bid_price: [&str; 15] = ["100.00"; 15];
238+
let ask_price: [&str; 15] = [
239+
"100.01", "100.02", "100.03", "100.04", "100.05", "100.06", "100.07", "100.08",
240+
"100.09", "100.10", "100.11", "100.12", "100.13", "100.14", "100.15",
241+
];
242+
243+
for i in 0..15 {
244+
spread_analyzer_10.handle_quote(&stub_quote(bid_price[i], ask_price[i]));
245+
}
246+
247+
assert_eq!(spread_analyzer_10.spreads.len(), 10);
248+
assert!((spread_analyzer_10.average - 0.105).abs() < 1e-9);
216249
}
217250

218251
#[rstest]

crates/indicators/src/volatility/fuzzy.rs

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -324,6 +324,18 @@ impl FuzzyCandlesticks {
324324
self.last_low = low;
325325

326326
let total = (high - low).abs();
327+
328+
// Bound the rolling windows to `period`, matching the Cython
329+
// `deque(maxlen=period)`. Without this the fixed-capacity deques grow to
330+
// their 1024 capacity, so the means (sum / period) and standard
331+
// deviations are computed over far more than `period` candles.
332+
if self.lengths.len() == self.period {
333+
self.lengths.pop_front();
334+
self.body_percents.pop_front();
335+
self.upper_wick_percents.pop_front();
336+
self.lower_wick_percents.pop_front();
337+
}
338+
327339
let _ = self.lengths.push_back(total);
328340

329341
if total == 0.0 {
@@ -617,6 +629,42 @@ mod tests {
617629
assert_eq!(fuzzy_candlesticks_10.vector, expected_vec);
618630
}
619631

632+
#[rstest]
633+
fn test_windows_bounded_to_period(mut fuzzy_candlesticks_10: FuzzyCandlesticks) {
634+
// Regression: the four rolling windows must stay bounded to `period`
635+
// (matching the Cython `deque(maxlen=period)`). Previously the
636+
// fixed-capacity deques grew to their 1024 capacity, so the means
637+
// (sum / period) and standard deviations were computed over far more than
638+
// `period` candles.
639+
let bars = [
640+
(150.25, 153.4, 148.1, 152.75),
641+
(152.8, 155.2, 151.3, 151.95),
642+
(151.9, 152.85, 147.6, 148.2),
643+
(148.3, 150.75, 146.9, 150.4),
644+
(150.5, 154.3, 149.8, 153.9),
645+
(153.95, 155.8, 152.2, 152.6),
646+
(152.7, 153.4, 148.5, 149.1),
647+
(149.2, 151.9, 147.3, 151.5),
648+
(151.6, 156.4, 151.0, 155.8),
649+
(155.9, 157.2, 153.7, 154.3),
650+
(154.3, 158.0, 153.0, 157.2),
651+
(157.2, 159.5, 155.1, 156.0),
652+
(156.0, 156.9, 152.4, 153.1),
653+
(153.1, 155.0, 150.2, 154.8),
654+
(154.8, 157.7, 154.0, 156.9),
655+
];
656+
657+
for (open, high, low, close) in bars {
658+
fuzzy_candlesticks_10.update_raw(open, high, low, close);
659+
}
660+
661+
assert!(fuzzy_candlesticks_10.initialized());
662+
assert_eq!(fuzzy_candlesticks_10.lengths.len(), 10);
663+
assert_eq!(fuzzy_candlesticks_10.body_percents.len(), 10);
664+
assert_eq!(fuzzy_candlesticks_10.upper_wick_percents.len(), 10);
665+
assert_eq!(fuzzy_candlesticks_10.lower_wick_percents.len(), 10);
666+
}
667+
620668
#[rstest]
621669
fn test_reset(mut fuzzy_candlesticks_10: FuzzyCandlesticks) {
622670
fuzzy_candlesticks_10.update_raw(151.6, 156.4, 151.0, 155.8);

crates/indicators/src/volatility/rvi.rs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,14 @@ impl RelativeVolatilityIndex {
131131
}
132132

133133
pub fn update_raw(&mut self, close: f64) {
134+
// Bound the price window to `period`, matching the Cython
135+
// `deque(maxlen=period)`. The fixed-capacity deque otherwise retains up
136+
// to 1024 prices, so the standard deviation below is computed over far
137+
// more than `period` observations while using a `period`-window mean.
138+
if self.prices.len() == self.period {
139+
self.prices.pop_front();
140+
}
141+
134142
self.prices.push_back(close);
135143
self.ma.update_raw(close);
136144

@@ -223,6 +231,21 @@ mod tests {
223231
assert_eq!(rvi_10.value, 10.0);
224232
}
225233

234+
#[rstest]
235+
fn test_prices_window_bounded_to_period(mut rvi_10: RelativeVolatilityIndex) {
236+
// Regression: the price window must stay bounded to `period` (matching the
237+
// Cython `deque(maxlen=period)`). Previously the fixed-capacity deque grew
238+
// to its 1024 capacity, so the standard deviation was computed over far
239+
// more than `period` prices while using a `period`-window mean.
240+
for i in 0..50 {
241+
rvi_10.update_raw(100.0 + f64::from(i));
242+
}
243+
244+
assert!(rvi_10.initialized());
245+
assert_eq!(rvi_10.prices.len(), 10);
246+
assert_eq!(rvi_10.value, 10.0);
247+
}
248+
226249
#[rstest]
227250
fn test_reset_successfully_returns_indicator_to_fresh_state(
228251
mut rvi_10: RelativeVolatilityIndex,

0 commit comments

Comments
 (0)