caio.co/de/go-tdigest

Properly compute extreme CDFs

When we reach the last centroid in the summary, what we want to
do is assume that the last two centroids are of equal width
then estimate the CDF via a simple interpolation. Before this
patch we would wrongly bound (and compute) this estimation with
the last item instead of the one before the last.

Fixes #17
Id
0ff90c8eb0c889e7135830d73683902c73442028
Author
Caio
Commit time
2018-01-18T18:04:13+01:00

Modified tdigest.go

@@ -280,11 +280,12
right = (t.summary.Mean(i+1) - t.summary.Mean(i)) / 2
}

- // last centroid
- lastMean := t.summary.Mean(t.summary.Len() - 1)
- if value < lastMean+right {
- lastCount := float64(t.summary.Count(t.summary.Len() - 1))
- return (tot + lastCount*interpolate(value, lastMean-left, lastMean+right)) / 2
+ // last centroid, the summary length is at least two
+ aIdx := t.summary.Len() - 2
+ aMean := t.summary.Mean(aIdx)
+ if value < aMean+right {
+ aCount := float64(t.summary.Count(aIdx))
+ return (tot + aCount*interpolate(value, aMean-left, aMean+right)) / 2
}
return 1
}

Modified tdigest_test.go

@@ -431,6 +431,13
if softErrors >= 3 {
t.Errorf("Too many soft errors")
}
+
+ // Issue #17, verify that we are hitting the extreme CDF case
+ // XXX Maybe test this properly instead of having a hardcoded value
+ extreme := digest.CDF(0.71875)
+ if !closeEnough(extreme, 1) {
+ t.Errorf("Expected something close to 1 but got %.4f instead", extreme)
+ }
}

func shouldPanic(f func(), t *testing.T, message string) {