Skip to content

Commit 21b183f

Browse files
committed
assertion: address NaN issue
When a timeseries is constant zero, the computations were previously resulting in NaNs, which we don't want here.
1 parent 4d8f17c commit 21b183f

File tree

2 files changed

+21
-10
lines changed

2 files changed

+21
-10
lines changed

pkg/kv/kvserver/asim/assertion/assert.go

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -128,10 +128,28 @@ func (sa SteadyStateAssertion) Assert(
128128
max, _ := stats.Max(trimmedStoreStats)
129129
min, _ := stats.Min(trimmedStoreStats)
130130

131-
maxMean := math.Abs(max/mean - 1)
132-
minMean := math.Abs(min/mean - 1)
131+
var maxMean, minMean float64
132+
if mean == 0 {
133+
if min == 0 || max == 0 {
134+
// If the min is zero, all datapoints are nonnegative, so for the mean
135+
// to be zero, they must all be zero. If the max is zero, vice versa.
136+
// Define 0/0=1 to capture that they're equal, which is what matters
137+
// here (0/0 defaults to NaN in Go, but we don't want that here).
138+
maxMean = 0
139+
minMean = 0
140+
} else {
141+
// The datapoints cross zero, and their mean is zero, for example
142+
// [-1, 1]. The values here would also result from the "regular"
143+
// computation below, but it doesn't hurt to be explicit.
144+
minMean = math.Inf(1)
145+
maxMean = math.Inf(1)
146+
}
147+
} else {
148+
maxMean = math.Abs(max/mean - 1)
149+
minMean = math.Abs(min/mean - 1)
150+
}
133151

134-
if sa.Threshold.isViolated(maxMean) || sa.Threshold.isViolated(minMean) {
152+
if sa.Threshold.isViolated(maxMean) || sa.Threshold.isViolated(minMean) || math.IsNaN(maxMean) || math.IsNaN(minMean) {
135153
if holds {
136154
fmt.Fprintf(&buf, " %s\n", sa)
137155
holds = false

pkg/kv/kvserver/asim/tests/testdata/non_rand/example_multi_store.txt

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ assertion stat=leases type=steady ticks=6 exact_bound=0
1919
----
2020
asserting: |leases(t)/mean_{T}(leases) - 1| = 0.00 ∀ t∈T and each store (T=last 6 ticks)
2121

22-
# TODO(tbg): fix the NaN issue here.
2322
eval duration=5m seed=42 metrics=(leases) cfgs=(sma-count,mma-only)
2423
----
2524
leases#1: first: [s1=8, s2=3, s3=0, s4=0, s5=0, s6=0, s7=0, s8=0, s9=1, s10=0, s11=1, s12=0, s13=1, s14=0] (stddev=2.10, mean=1.00, sum=14)
@@ -36,9 +35,3 @@ failed assertion sample 1
3635
max/mean=4.00 tick=3
3736
max/mean=4.00 tick=4
3837
max/mean=4.00 tick=5
39-
steady state stat=leases threshold=(=0.00) ticks=6
40-
store=3 min/mean=NaN max/mean=NaN
41-
store=4 min/mean=NaN max/mean=NaN
42-
store=5 min/mean=NaN max/mean=NaN
43-
store=7 min/mean=NaN max/mean=NaN
44-
store=8 min/mean=NaN max/mean=NaN

0 commit comments

Comments
 (0)