Skip to content

Commit 6b7b84f

Browse files
authored
[ML] Fix failing change point aggregation tests (#104819)
This fixes a number of failures which have accumulated in the change point aggregation tests: 1. Tests on FP and TP rate are sensitive to exact random numbers so seeding. All the failures have been by small margins, but it is annoying to have them fail periodically. I've made the tests much less sensitive so they should have close to zero chance of failure now. 2. FPs for the slope direction tests are causing test failures. These can be triggered by different random numbers. Since these tests really only care that we identify the correct slope direction I've stopped the FP path triggering a failure. Closes #103847 Closes #103848 Closes #103926 Closes #104798 Closes #104804
1 parent 47b885a commit 6b7b84f

File tree

1 file changed

+27
-22
lines changed

1 file changed

+27
-22
lines changed

x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/aggs/changepoint/ChangePointAggregatorTests.java

Lines changed: 27 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -60,31 +60,30 @@ public void testStationaryFalsePositiveRate() throws IOException {
6060
int fp = 0;
6161
for (int i = 0; i < 100; i++) {
6262
double[] bucketValues = DoubleStream.generate(() -> 10 + normal.sample()).limit(40).toArray();
63-
ChangePointAggregator.TestStats test = ChangePointAggregator.testForChange(bucketValues, 1e-3);
63+
ChangePointAggregator.TestStats test = ChangePointAggregator.testForChange(bucketValues, 1e-4);
6464
fp += test.type() == ChangePointAggregator.Type.STATIONARY ? 0 : 1;
6565
}
66-
assertThat(fp, lessThan(5));
66+
assertThat(fp, lessThan(10));
6767

6868
fp = 0;
6969
GammaDistribution gamma = new GammaDistribution(RandomGeneratorFactory.createRandomGenerator(Randomness.get()), 1, 2);
7070
for (int i = 0; i < 100; i++) {
7171
double[] bucketValues = DoubleStream.generate(() -> gamma.sample()).limit(40).toArray();
72-
ChangePointAggregator.TestStats test = ChangePointAggregator.testForChange(bucketValues, 1e-3);
72+
ChangePointAggregator.TestStats test = ChangePointAggregator.testForChange(bucketValues, 1e-4);
7373
fp += test.type() == ChangePointAggregator.Type.STATIONARY ? 0 : 1;
7474
}
75-
assertThat(fp, lessThan(5));
75+
assertThat(fp, lessThan(10));
7676
}
7777

78-
@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/103848")
7978
public void testSampledDistributionTestFalsePositiveRate() throws IOException {
8079
NormalDistribution normal = new NormalDistribution(RandomGeneratorFactory.createRandomGenerator(Randomness.get()), 0.0, 1.0);
8180
int fp = 0;
8281
for (int i = 0; i < 100; i++) {
8382
double[] bucketValues = DoubleStream.generate(() -> 10 + normal.sample()).limit(5000).toArray();
84-
ChangePointAggregator.TestStats test = ChangePointAggregator.testForChange(bucketValues, 0.05);
83+
ChangePointAggregator.TestStats test = ChangePointAggregator.testForChange(bucketValues, 1e-4);
8584
fp += test.type() == ChangePointAggregator.Type.STATIONARY ? 0 : 1;
8685
}
87-
assertThat(fp, lessThan(5));
86+
assertThat(fp, lessThan(10));
8887
}
8988

9089
public void testNonStationaryFalsePositiveRate() throws IOException {
@@ -93,23 +92,22 @@ public void testNonStationaryFalsePositiveRate() throws IOException {
9392
for (int i = 0; i < 100; i++) {
9493
AtomicInteger j = new AtomicInteger();
9594
double[] bucketValues = DoubleStream.generate(() -> j.incrementAndGet() + normal.sample()).limit(40).toArray();
96-
ChangePointAggregator.TestStats test = ChangePointAggregator.testForChange(bucketValues, 1e-3);
95+
ChangePointAggregator.TestStats test = ChangePointAggregator.testForChange(bucketValues, 1e-4);
9796
fp += test.type() == ChangePointAggregator.Type.NON_STATIONARY ? 0 : 1;
9897
}
99-
assertThat(fp, lessThan(5));
98+
assertThat(fp, lessThan(10));
10099

101100
fp = 0;
102101
GammaDistribution gamma = new GammaDistribution(RandomGeneratorFactory.createRandomGenerator(Randomness.get()), 1, 2);
103102
for (int i = 0; i < 100; i++) {
104103
AtomicInteger j = new AtomicInteger();
105104
double[] bucketValues = DoubleStream.generate(() -> j.incrementAndGet() + gamma.sample()).limit(40).toArray();
106-
ChangePointAggregator.TestStats test = ChangePointAggregator.testForChange(bucketValues, 1e-3);
105+
ChangePointAggregator.TestStats test = ChangePointAggregator.testForChange(bucketValues, 1e-4);
107106
fp += test.type() == ChangePointAggregator.Type.NON_STATIONARY ? 0 : 1;
108107
}
109-
assertThat(fp, lessThan(5));
108+
assertThat(fp, lessThan(10));
110109
}
111110

112-
@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/103847")
113111
public void testStepChangePower() throws IOException {
114112
NormalDistribution normal = new NormalDistribution(RandomGeneratorFactory.createRandomGenerator(Randomness.get()), 0, 2);
115113
int tp = 0;
@@ -121,7 +119,7 @@ public void testStepChangePower() throws IOException {
121119
ChangePointAggregator.TestStats test = ChangePointAggregator.testForChange(bucketValues, 0.05);
122120
tp += test.type() == ChangePointAggregator.Type.STEP_CHANGE ? 1 : 0;
123121
}
124-
assertThat(tp, greaterThan(90));
122+
assertThat(tp, greaterThan(80));
125123

126124
tp = 0;
127125
GammaDistribution gamma = new GammaDistribution(RandomGeneratorFactory.createRandomGenerator(Randomness.get()), 1, 2);
@@ -133,7 +131,7 @@ public void testStepChangePower() throws IOException {
133131
ChangePointAggregator.TestStats test = ChangePointAggregator.testForChange(bucketValues, 0.05);
134132
tp += test.type() == ChangePointAggregator.Type.STEP_CHANGE ? 1 : 0;
135133
}
136-
assertThat(tp, greaterThan(90));
134+
assertThat(tp, greaterThan(80));
137135
}
138136

139137
public void testTrendChangePower() throws IOException {
@@ -148,7 +146,7 @@ public void testTrendChangePower() throws IOException {
148146
ChangePointAggregator.TestStats test = ChangePointAggregator.testForChange(bucketValues, 0.05);
149147
tp += test.type() == ChangePointAggregator.Type.TREND_CHANGE ? 1 : 0;
150148
}
151-
assertThat(tp, greaterThan(90));
149+
assertThat(tp, greaterThan(80));
152150

153151
tp = 0;
154152
GammaDistribution gamma = new GammaDistribution(RandomGeneratorFactory.createRandomGenerator(Randomness.get()), 1, 2);
@@ -161,7 +159,7 @@ public void testTrendChangePower() throws IOException {
161159
ChangePointAggregator.TestStats test = ChangePointAggregator.testForChange(bucketValues, 0.05);
162160
tp += test.type() == ChangePointAggregator.Type.TREND_CHANGE ? 1 : 0;
163161
}
164-
assertThat(tp, greaterThan(90));
162+
assertThat(tp, greaterThan(80));
165163
}
166164

167165
public void testDistributionChangeTestPower() throws IOException {
@@ -253,25 +251,32 @@ public void testConstant() throws IOException {
253251
);
254252
}
255253

256-
@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/103926")
257254
public void testSlopeUp() throws IOException {
258255
NormalDistribution normal = new NormalDistribution(RandomGeneratorFactory.createRandomGenerator(Randomness.get()), 0, 2);
259256
AtomicInteger i = new AtomicInteger();
260257
double[] bucketValues = DoubleStream.generate(() -> i.addAndGet(1) + normal.sample()).limit(40).toArray();
261258
testChangeType(bucketValues, changeType -> {
262-
assertThat(changeType, instanceOf(ChangeType.NonStationary.class));
263-
assertThat(Arrays.toString(bucketValues), ((ChangeType.NonStationary) changeType).getTrend(), equalTo("increasing"));
259+
if (changeType instanceof ChangeType.NonStationary) {
260+
assertThat(Arrays.toString(bucketValues), ((ChangeType.NonStationary) changeType).getTrend(), equalTo("increasing"));
261+
} else {
262+
// Handle infrequent false positives.
263+
assertThat(changeType, instanceOf(ChangeType.TrendChange.class));
264+
}
265+
264266
});
265267
}
266268

267-
@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/104798")
268269
public void testSlopeDown() throws IOException {
269270
NormalDistribution normal = new NormalDistribution(RandomGeneratorFactory.createRandomGenerator(Randomness.get()), 0, 2);
270271
AtomicInteger i = new AtomicInteger(40);
271272
double[] bucketValues = DoubleStream.generate(() -> i.decrementAndGet() + normal.sample()).limit(40).toArray();
272273
testChangeType(bucketValues, changeType -> {
273-
assertThat(changeType, instanceOf(ChangeType.NonStationary.class));
274-
assertThat(Arrays.toString(bucketValues), ((ChangeType.NonStationary) changeType).getTrend(), equalTo("decreasing"));
274+
if (changeType instanceof ChangeType.NonStationary) {
275+
assertThat(Arrays.toString(bucketValues), ((ChangeType.NonStationary) changeType).getTrend(), equalTo("decreasing"));
276+
} else {
277+
// Handle infrequent false positives.
278+
assertThat(changeType, instanceOf(ChangeType.TrendChange.class));
279+
}
275280
});
276281
}
277282

0 commit comments

Comments
 (0)