Skip to content

Commit 92adbd4

Browse files
committed
hdr: fix observe_batch alert gate and large-moment variance fallback
Two defects in the code recovered from #325, both found in review: - observe_batch gated its drift alert on `old_n > 10`, while observe counts the new element first and tests `> 10`, i.e. >= 10 prior. At exactly 10 prior observations, observe_batch(&[x]) stayed silent where observe(x) alerted. Gate is now `old_n >= 10`. - MomentsU32::variance's fallback (when n*sum_sq or sum^2 overflows u128) subtracted two ~2^64-scale f64 values and could cancel the variance to 0 (2^33 values split between u32::MAX and u32::MAX-1: 0.0 instead of 0.25). It now centres on q = floor(sum/n) exactly in u128 first, so the float step works on variance-sized quantities. Each fix has a regression test that failed before the change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019HnekoM1EidTwQLS3oFVFm
1 parent 1c31868 commit 92adbd4

2 files changed

Lines changed: 56 additions & 6 deletions

File tree

‎src/hpc/cascade.rs‎

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -220,8 +220,9 @@ impl Cascade {
220220
///
221221
/// Drift is judged once per batch, not per element: an alert fires when
222222
/// the batch moves μ by more than 2σ of the pre-batch state (and the state
223-
/// had > 10 observations and σ > 0), the same test `observe` applies to a
224-
/// single step.
223+
/// had at least 10 observations and σ > 0), the same test `observe`
224+
/// applies to a single step, so `observe_batch(&[x])` alerts exactly when
225+
/// `observe(x)` does.
225226
pub fn observe_batch(&mut self, distances: &[u32]) -> Option<ShiftAlert> {
226227
let b = crate::hpc::statistics::moments_u32(distances);
227228
if b.n == 0 {
@@ -245,7 +246,7 @@ impl Cascade {
245246
}
246247
self.observations = old_n + b.n as usize;
247248

248-
if old_n > 10 && old_sigma > 0.0 && (self.mu - old_mu).abs() > 2.0 * old_sigma {
249+
if old_n >= 10 && old_sigma > 0.0 && (self.mu - old_mu).abs() > 2.0 * old_sigma {
249250
Some(ShiftAlert {
250251
old_mu,
251252
new_mu: self.mu,
@@ -888,6 +889,25 @@ mod tests {
888889
assert!(alert.new_mu > alert.old_mu + 2.0 * alert.old_sigma);
889890
}
890891

892+
/// A singleton batch is the same event as one `observe` call, so both
893+
/// must agree on the alert gate at the boundary: exactly 10 prior
894+
/// observations, then an outlier. `observe` counts the new element
895+
/// before testing `> 10`; `observe_batch` must admit the same state.
896+
#[test]
897+
fn observe_batch_singleton_matches_observe_alert_gate() {
898+
let mut seq = Cascade::from_threshold(8000, 2048);
899+
let mut batch = Cascade::from_threshold(8000, 2048);
900+
for i in 0..10u32 {
901+
assert!(seq.observe(8000 + 100 * (i % 2)).is_none());
902+
assert!(batch.observe(8000 + 100 * (i % 2)).is_none());
903+
}
904+
assert_eq!(seq.observations(), 10);
905+
let a = seq.observe(20000);
906+
let b = batch.observe_batch(&[20000]);
907+
assert!(a.is_some(), "observe must alert on the outlier");
908+
assert!(b.is_some(), "observe_batch(&[x]) must alert exactly like observe(x)");
909+
}
910+
891911
#[test]
892912
fn packed_database_roundtrip() {
893913
let vec_bytes = 256;

‎src/hpc/statistics.rs‎

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -410,7 +410,9 @@ impl MomentsU32 {
410410
/// Computed as `(n·Σx² − (Σx)²) / n²`. The numerator is formed exactly in
411411
/// `u128` whenever it fits (always, for Hamming-scale data: n ≤ 2³², x ≤
412412
/// 2¹⁷), so there is no cancellation between two large floats; only the
413-
/// final division rounds. Past that range it falls back to `f64`.
413+
/// final division rounds. Past that range it centres the sums on the
414+
/// integer part of the mean in `u128` first, so the `f64` step still
415+
/// works on small, variance-sized quantities.
414416
pub fn variance(&self) -> f64 {
415417
if self.n == 0 {
416418
return 0.0;
@@ -419,8 +421,18 @@ impl MomentsU32 {
419421
match (n.checked_mul(self.sum_sq), self.sum.checked_mul(self.sum)) {
420422
(Some(a), Some(b)) => (a - b) as f64 / (self.n as f64 * self.n as f64),
421423
_ => {
422-
let mean = self.mean();
423-
(self.sum_sq as f64 / self.n as f64 - mean * mean).max(0.0)
424+
// Centre on the integer part of the mean, q = ⌊Σx / n⌋, with
425+
// remainder r = Σx − n·q < n. Then, exactly in u128,
426+
// Σ(x − q)² = Σx² − q·Σx − q·r, which is < n·2⁶⁴ and never
427+
// negative at any step. The true M2 is that minus r²/n, and
428+
// both terms are O(n·(σ² + 1)), so the one float subtraction
429+
// cannot cancel the variance away.
430+
let q = self.sum / n;
431+
let r = self.sum % n;
432+
let centred = self.sum_sq - q * self.sum - q * r;
433+
let rf = r as f64;
434+
let nf = self.n as f64;
435+
((centred as f64 - rf * rf / nf) / nf).max(0.0)
424436
}
425437
}
426438
}
@@ -481,6 +493,24 @@ pub fn moments_u32(values: &[u32]) -> MomentsU32 {
481493
mod moments_tests {
482494
use super::*;
483495

496+
/// Past the exact-`u128` range the variance must not cancel: 2³³ values
497+
/// split evenly between `u32::MAX` and `u32::MAX - 1` have variance
498+
/// exactly 0.25, and `n·Σx²` overflows `u128`, so this takes the
499+
/// fallback path.
500+
#[test]
501+
fn variance_fallback_does_not_cancel() {
502+
let half = 1u128 << 32;
503+
let hi = u128::from(u32::MAX);
504+
let lo = hi - 1;
505+
let m = MomentsU32 {
506+
n: 1u64 << 33,
507+
sum: half * (hi + lo),
508+
sum_sq: half * (hi * hi + lo * lo),
509+
};
510+
assert!(u128::from(m.n).checked_mul(m.sum_sq).is_none(), "fixture must take the fallback");
511+
assert!((m.variance() - 0.25).abs() < 1e-9, "variance {}", m.variance());
512+
}
513+
484514
fn xorshift(n: usize, mut s: u64, mask: u32) -> Vec<u32> {
485515
(0..n)
486516
.map(|_| {

0 commit comments

Comments
 (0)