Skip to content

Commit

Permalink
[ML] Fix failing change point aggregation tests (#104819)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
tveasey committed Jan 26, 2024
1 parent 47b885a commit 6b7b84f
Showing 1 changed file with 27 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -60,31 +60,30 @@ public void testStationaryFalsePositiveRate() throws IOException {
int fp = 0;
for (int i = 0; i < 100; i++) {
double[] bucketValues = DoubleStream.generate(() -> 10 + normal.sample()).limit(40).toArray();
ChangePointAggregator.TestStats test = ChangePointAggregator.testForChange(bucketValues, 1e-3);
ChangePointAggregator.TestStats test = ChangePointAggregator.testForChange(bucketValues, 1e-4);
fp += test.type() == ChangePointAggregator.Type.STATIONARY ? 0 : 1;
}
assertThat(fp, lessThan(5));
assertThat(fp, lessThan(10));

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

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

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

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

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

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

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

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

public void testDistributionChangeTestPower() throws IOException {
Expand Down Expand Up @@ -253,25 +251,32 @@ public void testConstant() throws IOException {
);
}

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

});
}

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

Expand Down

0 comments on commit 6b7b84f

Please sign in to comment.