[pinpoint] handle all values equal edge case in ComparePairwise
When every element of valuesA is identical and every element of valuesB
is identical, the wilcoxon signed rank test is unable to estimate a
confidence interval. However, the underlying data is still useful. We
nudge one of the values by an insignificant amount to unblock the
analysis and return a valid confidence interval.
This edge case is not supported in catapult and will return an error,
blocking bisection.
Bug: b/332612913
Change-Id: I90e1397cd2e5dd04a0a22420caa311d13e7945af
Reviewed-on: https://skia-review.googlesource.com/c/buildbot/+/862501
Reviewed-by: Kaylee Lubick <kjlubick@google.com>
Commit-Queue: Leina Sun <sunxiaodi@google.com>
diff --git a/pinpoint/go/workflows/internal/BUILD.bazel b/pinpoint/go/workflows/internal/BUILD.bazel
index 71167e5..67c6915 100644
--- a/pinpoint/go/workflows/internal/BUILD.bazel
+++ b/pinpoint/go/workflows/internal/BUILD.bazel
@@ -24,6 +24,7 @@
"//go/auth",
"//go/httputils",
"//go/skerr",
+ "//go/sklog",
"//pinpoint/go/backends",
"//pinpoint/go/bot_configs",
"//pinpoint/go/build_chrome",
diff --git a/pinpoint/go/workflows/internal/compare.go b/pinpoint/go/workflows/internal/compare.go
index c084c99..449c959 100644
--- a/pinpoint/go/workflows/internal/compare.go
+++ b/pinpoint/go/workflows/internal/compare.go
@@ -4,6 +4,7 @@
"context"
"go.skia.org/infra/go/skerr"
+ "go.skia.org/infra/go/sklog"
"go.skia.org/infra/pinpoint/go/compare"
"go.skia.org/infra/pinpoint/go/midpoint"
"go.temporal.io/sdk/workflow"
@@ -12,6 +13,7 @@
const (
functional = "Functional"
performance = "Performance"
+ nudgeFactor = float64(1e-10)
)
type CommitPairValues struct {
@@ -108,5 +110,33 @@
// ComparePairwiseActivity wraps compare.ComparePairwise as a temporal activity
func ComparePairwiseActivity(ctx context.Context, valuesA, valuesB []float64, dir compare.ImprovementDir) (*compare.ComparePairwiseResult, error) {
+ valuesB = handlePairwiseEdgeCase(valuesA, valuesB)
return compare.ComparePairwise(valuesA, valuesB, dir)
}
+
+// if every value in valuesA is identicial and every value in valuesB is identical,
+// pairwise comparison will fail to return a confidence interval because the
+// wilcoxon_signed_rank.go runs into an error:
+// "cannot compute confidence interval when all observations are zero or tied"
+// This edge case can happen for some very consistent, near-deterministic benchmark runs.
+// However, this does not mean that the data collected is not useful. Only one data point
+// needs to be nudged to return a confidence interval. Here we manipulate the data
+// on a significant figure that should not have any adverse affect on the overall sample
+func handlePairwiseEdgeCase(valuesA, valuesB []float64) []float64 {
+ allSameA := allSameValues(valuesA)
+ allSameB := allSameValues(valuesB)
+ if allSameA && allSameB {
+ sklog.Warningf("all values in A are identical and all values in B are identical. Nudging one element by %v in valuesB. ValuesA: %v; ValuesB: %v", valuesB[0]*nudgeFactor, valuesA, valuesB)
+ valuesB[0] += valuesB[0] * nudgeFactor
+ }
+ return valuesB
+}
+
+func allSameValues(values []float64) bool {
+ for i := 1; i < len(values); i++ {
+ if values[i] != values[0] {
+ return false
+ }
+ }
+ return true
+}
diff --git a/pinpoint/go/workflows/internal/compare_test.go b/pinpoint/go/workflows/internal/compare_test.go
index 4ed2614..9b293ed 100644
--- a/pinpoint/go/workflows/internal/compare_test.go
+++ b/pinpoint/go/workflows/internal/compare_test.go
@@ -1,6 +1,7 @@
package internal
import (
+ "math"
"testing"
"github.com/stretchr/testify/assert"
@@ -175,3 +176,59 @@
assert.Equal(t, expectedFunc, actual.OtherResult)
assert.Equal(t, performance, actual.ResultType)
}
+
+func TestComparePairwise_GivenSimpleValues_ReturnsResult(t *testing.T) {
+ valuesA := []float64{8491008, 8491008, 8491008, 8491008, 8491008, 8491008, 8491008, 8491008, 8491008, 8491008}
+ valuesB := []float64{14225408, 14225408, 14225408, 14225408, 14225408, 14225408, 14225408, 14225408, 14225408, 14225408}
+
+ // demonstrate the edge case
+ nanPerf, err := compare.ComparePairwise(valuesA, valuesB, compare.UnknownDir)
+ require.NoError(t, err)
+ require.True(t, math.IsNaN(nanPerf.PairwiseWilcoxonSignedRankedTestResult.LowerCi))
+ require.True(t, math.IsNaN(nanPerf.PairwiseWilcoxonSignedRankedTestResult.UpperCi))
+
+ valuesB = handlePairwiseEdgeCase(valuesA, valuesB)
+ expectedPerf, err := compare.ComparePairwise(valuesA, valuesB, compare.UnknownDir)
+ require.NoError(t, err)
+ require.False(t, math.IsNaN(expectedPerf.PairwiseWilcoxonSignedRankedTestResult.LowerCi))
+ require.False(t, math.IsNaN(expectedPerf.PairwiseWilcoxonSignedRankedTestResult.UpperCi))
+
+ testSuite := &testsuite.WorkflowTestSuite{}
+ env := testSuite.NewTestActivityEnvironment()
+ env.RegisterActivity(ComparePairwiseActivity)
+ res, err := env.ExecuteActivity(ComparePairwiseActivity, valuesA, valuesB, compare.UnknownDir)
+ require.NoError(t, err)
+
+ var actual *compare.ComparePairwiseResult
+ err = res.Get(&actual)
+ require.NoError(t, err)
+ assert.Equal(t, expectedPerf, actual)
+}
+
+func TestComparePairwise_GivenAllSameValues_ReturnsResult(t *testing.T) {
+ valuesA := []float64{8491008, 8491008, 8491008, 8491008, 8491008, 8491008, 8491008, 8491008, 8491008, 8491008}
+ valuesB := []float64{14225408, 14225408, 14225408, 14225408, 14225408, 14225408, 14225408, 14225408, 14225408, 14225408}
+
+ // demonstrate the edge case
+ nanPerf, err := compare.ComparePairwise(valuesA, valuesB, compare.UnknownDir)
+ require.NoError(t, err)
+ require.True(t, math.IsNaN(nanPerf.PairwiseWilcoxonSignedRankedTestResult.LowerCi))
+ require.True(t, math.IsNaN(nanPerf.PairwiseWilcoxonSignedRankedTestResult.UpperCi))
+
+ valuesB = handlePairwiseEdgeCase(valuesA, valuesB)
+ expectedPerf, err := compare.ComparePairwise(valuesA, valuesB, compare.UnknownDir)
+ require.NoError(t, err)
+ require.False(t, math.IsNaN(expectedPerf.PairwiseWilcoxonSignedRankedTestResult.LowerCi))
+ require.False(t, math.IsNaN(expectedPerf.PairwiseWilcoxonSignedRankedTestResult.UpperCi))
+
+ testSuite := &testsuite.WorkflowTestSuite{}
+ env := testSuite.NewTestActivityEnvironment()
+ env.RegisterActivity(ComparePairwiseActivity)
+ res, err := env.ExecuteActivity(ComparePairwiseActivity, valuesA, valuesB, compare.UnknownDir)
+ require.NoError(t, err)
+
+ var actual *compare.ComparePairwiseResult
+ err = res.Get(&actual)
+ require.NoError(t, err)
+ assert.Equal(t, expectedPerf, actual)
+}
