Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid inconsistent quantile computation between architectures #80

Merged
merged 1 commit into from
Jun 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions dataset/generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,3 +65,21 @@ func (g *Pareto) Generate() float64 {
r := rand.ExpFloat64() / g.shape
return math.Exp(math.Log(g.scale) + r)
}

// Linearly increasing stream, with zeroes once every 2 values.
type LinearWithZeroes struct {
currentVal float64
count int
}

func NewLinearWithZeroes() *LinearWithZeroes { return &LinearWithZeroes{0, 0} }

func (g *LinearWithZeroes) Generate() float64 {
g.count++
if g.count%2 == 0 {
value := g.currentVal
g.currentVal++
return value
}
return 0
}
8 changes: 7 additions & 1 deletion ddsketch/ddsketch.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,13 @@ func (s *DDSketch) GetValueAtQuantile(quantile float64) (float64, error) {
return math.NaN(), errEmptySketch
}

rank := quantile * (count - 1)
// Use an explicit floating point conversion (as per Go specification) to make sure that no
// "fused multiply and add" (FMA) operation is used in the following code subtracting values
// from `rank`. Not doing so can lead to inconsistent rounding and return value for this
// function, depending on the architecture and whether FMA operations are used or not by the
// compiler.
rank := float64(quantile * (count - 1))

negativeValueCount := s.negativeValueStore.TotalCount()
if rank < negativeValueCount {
return -s.Value(s.negativeValueStore.KeyAtRank(negativeValueCount - 1 - rank)), nil
Expand Down
14 changes: 13 additions & 1 deletion ddsketch/ddsketch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,11 @@ type testCase struct {
}

var (
// testSize=21 and testQuantiles=0.95 with the LinearWithZeroes generator exposes a bug if a
// "fused multiply and add" (FMA) operation is used in GetValueAtQuantile on ARM64, when the
// explicit floating point conversion is not used on the computed rank.
testQuantiles = []float64{0, 0.1, 0.25, 0.5, 0.75, 0.9, 0.95, 0.99, 0.999, 1}
testSizes = []int{3, 5, 10, 100, 1000}
testSizes = []int{3, 5, 10, 21, 100, 1000}
testCases = []testCase{
{
sketch: func() quantileSketch {
Expand Down Expand Up @@ -208,6 +211,15 @@ func TestLinear(t *testing.T) {
}
}

func TestLinearWithZeroes(t *testing.T) {
for _, testCase := range testCases {
for _, n := range testSizes {
linearWithZeroesGenerator := dataset.NewLinearWithZeroes()
evaluateSketch(t, n, linearWithZeroesGenerator, testCase.sketch(), testCase)
}
}
}

func TestNormal(t *testing.T) {
for _, testCase := range testCases {
for _, n := range testSizes {
Expand Down