Skip to content

Commit 19791e3

Browse files
authored
fix(core): use compensated summation for histograms (#1666)
## Summary Use modified Neumaier algorithm to calculate sums, counts, and quantiles for histogram samples. The naive `sum += value * weight` loop suffers catastrophic cancellation when the sample stream contains values of wildly different magnitudes. The classic Kahan/Peters counter-example `{1, +1e100, 1, -1e100}` evaluates to 0 with naive summation but to the correct 2.0 with the new algorithm. ## Change Type - [x] Bug fix - [ ] New feature - [ ] Non-functional (chore, refactoring, docs) - [ ] Performance ## How did you test this PR? Added unit tests to check correctness. ## References [Similar PR in Datadog-Agent](DataDog/datadog-agent#49913). Co-authored-by: mark.kirichenko <mark.kirichenko@datadoghq.com>
1 parent efb3648 commit 19791e3

4 files changed

Lines changed: 161 additions & 22 deletions

File tree

  • lib
    • saluki-components/src
    • saluki-core/src/data_model/event/metric/value

lib/saluki-components/src/destinations/prometheus/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -507,7 +507,7 @@ impl PrometheusHistogram {
507507

508508
fn merge_histogram(&mut self, histogram: &Histogram) {
509509
for sample in histogram.samples() {
510-
self.add_sample(sample.value.into_inner(), sample.weight);
510+
self.add_sample(sample.value.into_inner(), sample.weight.0 as u64);
511511
}
512512
}
513513

lib/saluki-components/src/encoders/datadog/metrics/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -959,7 +959,7 @@ fn encode_sketch_metric(
959959
// We convert histograms to sketches to be able to write them out in the payload.
960960
let mut ddsketch = DDSketch::default();
961961
for sample in histogram.samples() {
962-
ddsketch.insert_n(sample.value.into_inner(), sample.weight);
962+
ddsketch.insert_n(sample.value.into_inner(), sample.weight.0 as u64);
963963
}
964964

965965
write_dogsketch(output_stream, scratch_buf, packed_scratch_buf, timestamp, &ddsketch)?;

lib/saluki-components/src/transforms/aggregate/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -742,7 +742,7 @@ async fn transform_and_push_metric(
742742
.map(|(ts, hist)| {
743743
let mut sketch = DDSketch::default();
744744
for sample in hist.samples() {
745-
sketch.insert_n(sample.value.into_inner(), sample.weight);
745+
sketch.insert_n(sample.value.into_inner(), sample.weight.0 as u64);
746746
}
747747
(ts, sketch)
748748
})

lib/saluki-core/src/data_model/event/metric/value/histogram.rs

Lines changed: 158 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -17,23 +17,31 @@ pub struct WeightedSample {
1717
pub value: OrderedFloat<f64>,
1818

1919
/// The sample weight.
20-
pub weight: u64,
20+
pub weight: OrderedFloat<f64>,
2121
}
2222

2323
/// A basic histogram.
2424
#[derive(Clone, Debug, Default, Eq, PartialEq)]
2525
pub struct Histogram {
26-
sum: OrderedFloat<f64>,
2726
samples: SmallVec<[WeightedSample; 3]>,
27+
/// Weight shared by every sample so far; `0.0` means no samples have been inserted yet.
28+
shared_weight: OrderedFloat<f64>,
29+
/// Set to `true` on the first insertion whose weight differs from [`shared_weight`].
30+
weights_vary: bool,
2831
}
2932

3033
impl Histogram {
3134
/// Insert a sample into the histogram.
3235
pub fn insert(&mut self, value: f64, sample_rate: SampleRate) {
33-
self.sum += value * sample_rate.raw_weight();
36+
let weight = OrderedFloat(sample_rate.raw_weight());
37+
if self.shared_weight == OrderedFloat(0.0) {
38+
self.shared_weight = weight;
39+
} else if weight != self.shared_weight {
40+
self.weights_vary = true;
41+
}
3442
self.samples.push(WeightedSample {
3543
value: OrderedFloat(value),
36-
weight: sample_rate.weight(),
44+
weight,
3745
});
3846
}
3947

@@ -48,12 +56,32 @@ impl Histogram {
4856
// minimum and maximum, as well as quantile queries.
4957
self.samples.sort_unstable_by_key(|sample| sample.value);
5058

51-
let mut count = 0;
52-
let mut sum = 0.0;
53-
for sample in &self.samples {
54-
count += sample.weight;
55-
sum += sample.value.0 * sample.weight as f64;
56-
}
59+
// Compute count and sum in a single pass using compensated (Neumaier) summation.
60+
// Four accumulators: (count_s, count_c) for the weight total and (sum_s, sum_c) for the
61+
// value total, keeping one correction term per accumulator.
62+
let (count, sum) = if self.weights_vary {
63+
// Varying weights: accumulate value * weight for sum, weight for count.
64+
let (cs, cc, ss, sc) =
65+
self.samples
66+
.iter()
67+
.fold((0.0_f64, 0.0_f64, 0.0_f64, 0.0_f64), |(cs, cc, ss, sc), sample| {
68+
let (cs, cc) = neumaier_add(cs, cc, sample.weight.0);
69+
let (ss, sc) = neumaier_add(ss, sc, sample.value.0 * sample.weight.0);
70+
(cs, cc, ss, sc)
71+
});
72+
(cs + cc, ss + sc)
73+
} else {
74+
// Uniform weights: accumulate raw values for sum (scaled once at the end), weight for count.
75+
let (cs, cc, ss, sc) =
76+
self.samples
77+
.iter()
78+
.fold((0.0_f64, 0.0_f64, 0.0_f64, 0.0_f64), |(cs, cc, ss, sc), sample| {
79+
let (cs, cc) = neumaier_add(cs, cc, sample.weight.0);
80+
let (ss, sc) = neumaier_add(ss, sc, sample.value.0);
81+
(cs, cc, ss, sc)
82+
});
83+
(cs + cc, (ss + sc) * self.shared_weight.0)
84+
};
5785

5886
HistogramSummary {
5987
histogram: self,
@@ -64,7 +92,15 @@ impl Histogram {
6492

6593
/// Merges another histogram into this one.
6694
pub fn merge(&mut self, other: &mut Histogram) {
67-
self.sum += other.sum;
95+
if !self.weights_vary {
96+
if other.weights_vary {
97+
self.weights_vary = true;
98+
} else if self.shared_weight == OrderedFloat(0.0) {
99+
self.shared_weight = other.shared_weight;
100+
} else if other.shared_weight != OrderedFloat(0.0) && self.shared_weight != other.shared_weight {
101+
self.weights_vary = true;
102+
}
103+
}
68104
self.samples.extend(other.samples.drain(..));
69105
}
70106

@@ -77,16 +113,22 @@ impl Histogram {
77113
/// Summary view over a [`Histogram`].
78114
pub struct HistogramSummary<'a> {
79115
histogram: &'a Histogram,
80-
count: u64,
116+
count: f64,
81117
sum: f64,
82118
}
83119

84120
impl HistogramSummary<'_> {
85121
/// Returns the number of samples in the histogram.
86122
///
87123
/// This is adjusted by the weight of each sample, based on the sample rate given during insertion.
124+
///
125+
/// The underlying weight accumulation uses compensated (Neumaier) summation over float weights,
126+
/// and the result is rounded to the nearest integer. For standard sample rates whose reciprocals
127+
/// are exact integers (e.g. `0.1`, `0.25`, `0.5`, `1.0`) rounding has no effect; for
128+
/// non-integer-reciprocal rates (e.g. `0.21` → weight ≈ 4.762) it gives a closer approximation
129+
/// than truncation.
88130
pub fn count(&self) -> u64 {
89-
self.count
131+
self.count.round() as u64
90132
}
91133

92134
/// Returns the sum of all samples in the histogram.
@@ -114,7 +156,7 @@ impl HistogramSummary<'_> {
114156

115157
/// Returns the average value in the histogram.
116158
pub fn avg(&self) -> f64 {
117-
self.sum / self.count as f64
159+
self.sum / self.count
118160
}
119161

120162
/// Returns the median value in the histogram.
@@ -132,13 +174,14 @@ impl HistogramSummary<'_> {
132174
return None;
133175
}
134176

135-
let scaled_quantile = (quantile * 1000.0) as u64 / 10;
136-
let target = (scaled_quantile * self.count - 1) / 100;
177+
// target is the cumulative weight threshold: walk samples until the running weight exceeds it.
178+
let target = quantile * self.count - 0.01;
137179

138-
let mut weight = 0;
180+
let mut ws = 0.0_f64;
181+
let mut wc = 0.0_f64;
139182
for sample in &self.histogram.samples {
140-
weight += sample.weight;
141-
if weight > target {
183+
(ws, wc) = neumaier_add(ws, wc, sample.weight.0);
184+
if ws + wc > target {
142185
return Some(sample.value.0);
143186
}
144187
}
@@ -383,3 +426,99 @@ impl<'a> Iterator for HistogramIterRefMut<'a> {
383426
self.inner.next().map(|value| (value.timestamp, &mut value.value))
384427
}
385428
}
429+
430+
/// Performs a single Neumaier (compensated) addition step.
431+
///
432+
/// Returns the updated running sum `t` and the compensation term `c` that captures
433+
/// the rounding error lost when adding `x` to `s`.
434+
fn neumaier_add(s: f64, c: f64, x: f64) -> (f64, f64) {
435+
let t = s + x;
436+
let c = if s.abs() >= x.abs() {
437+
c + ((s - t) + x)
438+
} else {
439+
c + ((x - t) + s)
440+
};
441+
(t, c)
442+
}
443+
444+
#[cfg(test)]
445+
mod tests {
446+
use super::*;
447+
448+
fn histogram_from_values(values: &[(f64, u64)]) -> Histogram {
449+
let mut h = Histogram::default();
450+
for &(value, weight) in values {
451+
let weight = OrderedFloat(weight as f64);
452+
if h.shared_weight == OrderedFloat(0.0) {
453+
h.shared_weight = weight;
454+
} else if weight != h.shared_weight {
455+
h.weights_vary = true;
456+
}
457+
h.samples.push(WeightedSample {
458+
value: OrderedFloat(value),
459+
weight,
460+
});
461+
}
462+
h
463+
}
464+
465+
#[test]
466+
fn compensated_sum_catastrophic_cancellation() {
467+
// Naive summation: (1 + 1e100) + (1 - 1e100) = 0 due to float cancellation.
468+
// Compensated summation must return 2.0.
469+
let mut h = histogram_from_values(&[(1.0, 1), (1e100, 1), (1.0, 1), (-1e100, 1)]);
470+
let view = h.summary_view();
471+
assert_eq!(view.sum(), 2.0, "compensated sum should be 2.0, not 0.0");
472+
}
473+
474+
#[test]
475+
fn compensated_sum_empty() {
476+
let mut h = Histogram::default();
477+
let view = h.summary_view();
478+
assert_eq!(view.sum(), 0.0);
479+
assert_eq!(view.count(), 0);
480+
}
481+
482+
#[test]
483+
fn compensated_sum_uniform_weights_positives() {
484+
let mut h = histogram_from_values(&[(1.0, 2), (2.0, 2), (3.0, 2)]);
485+
let view = h.summary_view();
486+
// sum = (1+2+3)*2 = 12, count = 6
487+
assert_eq!(view.sum(), 12.0);
488+
assert_eq!(view.count(), 6);
489+
}
490+
491+
#[test]
492+
fn compensated_sum_uniform_weights_negatives() {
493+
let mut h = histogram_from_values(&[(-3.0, 1), (-2.0, 1), (-1.0, 1)]);
494+
let view = h.summary_view();
495+
assert_eq!(view.sum(), -6.0);
496+
assert_eq!(view.count(), 3);
497+
}
498+
499+
#[test]
500+
fn compensated_sum_varying_weights() {
501+
// Different weights trigger the fallback Neumaier path.
502+
let mut h = histogram_from_values(&[(1.0, 1), (2.0, 2), (3.0, 4)]);
503+
let view = h.summary_view();
504+
// sum = 1*1 + 2*2 + 3*4 = 1 + 4 + 12 = 17, count = 7
505+
assert_eq!(view.sum(), 17.0);
506+
assert_eq!(view.count(), 7);
507+
}
508+
509+
#[test]
510+
fn compensated_sum_all_zeros() {
511+
let mut h = histogram_from_values(&[(0.0, 1), (0.0, 1), (0.0, 1)]);
512+
let view = h.summary_view();
513+
assert_eq!(view.sum(), 0.0);
514+
assert_eq!(view.count(), 3);
515+
}
516+
517+
#[test]
518+
fn compensated_sum_single_value() {
519+
let mut h = histogram_from_values(&[(42.0, 5)]);
520+
let view = h.summary_view();
521+
assert_eq!(view.sum(), 210.0);
522+
assert_eq!(view.count(), 5);
523+
}
524+
}

0 commit comments

Comments
 (0)