Skip to content

Commit 54f7a82

Browse files
jaideeppyneclaude
andcommitted
fix: tdigest Quantile swaps interpolation weights and adds where the reference subtracts
The reference MergingDigest pairs mean[i] with the distance to the right edge of the bracket and mean[i+1] with the distance to the left edge. We passed them the other way round, so within a bracket a rising rank pulls the estimate toward the lower centroid and Quantile is not monotonic. The right tail branch used max + (...) where the reference uses max - (...), so it can return a value above MaxValue. Reachable through DecodeDouble, which accepts a tail centroid of weight above 1. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent a01e78c commit 54f7a82

2 files changed

Lines changed: 89 additions & 2 deletions

File tree

tdigest/double.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -376,7 +376,7 @@ func (d *Double) Quantile(rank float64) (float64, error) {
376376

377377
lastWeight := float64(d.centroids[len(d.centroids)-1].weight)
378378
if lastWeight > 1 && float64(d.centroidsWeight)-weight <= lastWeight/2.0 {
379-
return d.max + (float64(d.centroidsWeight)-weight-1.0)/(lastWeight/2.0-1.0)*(d.max-d.centroids[len(d.centroids)-1].mean), nil
379+
return d.max - (float64(d.centroidsWeight)-weight-1.0)/(lastWeight/2.0-1.0)*(d.max-d.centroids[len(d.centroids)-1].mean), nil
380380
}
381381

382382
// interpolate between extremes
@@ -401,7 +401,7 @@ func (d *Double) Quantile(rank float64) (float64, error) {
401401
}
402402
w1 := weight - weightSoFar - leftWeight
403403
w2 := weightSoFar + dw - weight - rightWeight
404-
return weightedAverage(d.centroids[i].mean, w1, d.centroids[i+1].mean, w2), nil
404+
return weightedAverage(d.centroids[i].mean, w2, d.centroids[i+1].mean, w1), nil
405405
}
406406
weightSoFar += dw
407407
}

tdigest/double_test.go

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package tdigest
1919

2020
import (
2121
"math"
22+
"math/rand"
2223
"testing"
2324

2425
"github.com/stretchr/testify/assert"
@@ -1023,3 +1024,89 @@ func TestDouble_SerializedSizeBytes(t *testing.T) {
10231024
assert.Equal(t, 1632, sizeWithoutBuffer)
10241025
})
10251026
}
1027+
1028+
func TestDouble_QuantileIsMonotonic(t *testing.T) {
1029+
distributions := map[string]func(r *rand.Rand) float64{
1030+
"gaussian": func(r *rand.Rand) float64 { return r.NormFloat64() },
1031+
"uniform": func(r *rand.Rand) float64 { return r.Float64() * 1000 },
1032+
"lognormal": func(r *rand.Rand) float64 { return math.Exp(r.NormFloat64()) },
1033+
}
1034+
1035+
for name, next := range distributions {
1036+
t.Run(name, func(t *testing.T) {
1037+
r := rand.New(rand.NewSource(1))
1038+
for _, k := range []uint16{10, 20, 50, 100, 200} {
1039+
for trial := 0; trial < 40; trial++ {
1040+
sketch, err := NewDouble(k)
1041+
assert.NoError(t, err)
1042+
n := 100 + r.Intn(50000)
1043+
for i := 0; i < n; i++ {
1044+
assert.NoError(t, sketch.Update(next(r)))
1045+
}
1046+
1047+
previousRank := 0.0
1048+
previousQuantile, err := sketch.Quantile(0)
1049+
assert.NoError(t, err)
1050+
for i := 1; i <= 1000; i++ {
1051+
rank := float64(i) / 1000.0
1052+
quantile, err := sketch.Quantile(rank)
1053+
assert.NoError(t, err)
1054+
assert.GreaterOrEqualf(t, quantile, previousQuantile,
1055+
"k=%d n=%d: Quantile(%v)=%v is below Quantile(%v)=%v",
1056+
k, n, rank, quantile, previousRank, previousQuantile)
1057+
previousRank = rank
1058+
previousQuantile = quantile
1059+
}
1060+
}
1061+
}
1062+
})
1063+
}
1064+
}
1065+
1066+
func TestDouble_QuantileIsWithinMinAndMax(t *testing.T) {
1067+
t.Run("From updates", func(t *testing.T) {
1068+
r := rand.New(rand.NewSource(2))
1069+
for trial := 0; trial < 100; trial++ {
1070+
sketch, err := NewDouble(DefaultK)
1071+
assert.NoError(t, err)
1072+
n := 10 + r.Intn(20000)
1073+
for i := 0; i < n; i++ {
1074+
assert.NoError(t, sketch.Update(r.NormFloat64()))
1075+
}
1076+
minValue, err := sketch.MinValue()
1077+
assert.NoError(t, err)
1078+
maxValue, err := sketch.MaxValue()
1079+
assert.NoError(t, err)
1080+
for i := 0; i <= 1000; i++ {
1081+
quantile, err := sketch.Quantile(float64(i) / 1000.0)
1082+
assert.NoError(t, err)
1083+
assert.GreaterOrEqual(t, quantile, minValue)
1084+
assert.LessOrEqual(t, quantile, maxValue)
1085+
}
1086+
}
1087+
})
1088+
1089+
// Update and Merge always leave both tail centroids as singletons, so the
1090+
// tail branches of Quantile are only reachable through deserialization.
1091+
t.Run("Decoded with heavy tail centroids", func(t *testing.T) {
1092+
sketch, err := newDoubleFromInternalStates(false, DefaultK, 0, 40,
1093+
[]doublePrecisionCentroid{{mean: 10, weight: 100}, {mean: 20, weight: 100}, {mean: 30, weight: 100}},
1094+
300, nil)
1095+
assert.NoError(t, err)
1096+
bytes, err := EncodeDouble(sketch, false)
1097+
assert.NoError(t, err)
1098+
decoded, err := DecodeDouble(bytes)
1099+
assert.NoError(t, err)
1100+
1101+
quantile, err := decoded.Quantile(0.9)
1102+
assert.NoError(t, err)
1103+
assert.InDelta(t, 34.081632653061224, quantile, 1e-12)
1104+
1105+
for i := 0; i <= 1000; i++ {
1106+
quantile, err := decoded.Quantile(float64(i) / 1000.0)
1107+
assert.NoError(t, err)
1108+
assert.GreaterOrEqual(t, quantile, 0.0)
1109+
assert.LessOrEqual(t, quantile, 40.0)
1110+
}
1111+
})
1112+
}

0 commit comments

Comments
 (0)