Merge pull request #80 from gduranceau/guillaume.duranceau/no-fma

Avoid inconsistent quantile computation between architectures

dataset/generator.go

@@ -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
+}

ddsketch/ddsketch.go

@@ -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

ddsketch/ddsketch_test.go

@@ -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 {
@@ -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 {