From 56461c008c4d394753733f57786f7e12570d92f0 Mon Sep 17 00:00:00 2001 From: Jason Whitmore Date: Tue, 27 May 2025 18:09:04 -0700 Subject: [PATCH 1/2] Changing KDE evaluate method to return correct ValueAndMagnitude object for bandwidth = 0 Prior to this change, the evaluate method would return a dummy object for an input with bandwidth = 0. This would occur if the dataset had zero variance (see KDE constructor). This would then cause ChangePointDetector to fail to detect a spike on a dataset containing all equal numbers except for one spike. This change removes the bandwidth = 0 condition for returning a dummy ValueAndMagnitude object. Statistical testing in ChangePointDetector now properly detects the spike in the example mentioned above. Unit tests are added to confirm the bug is fixed. --- .../xpack/ml/aggs/changepoint/KDE.java | 2 +- .../changepoint/ChangePointDetectorTests.java | 49 +++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) create mode 100644 x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/aggs/changepoint/ChangePointDetectorTests.java diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/changepoint/KDE.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/changepoint/KDE.java index 1a0342981c1d3..e0ac89a7efd35 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/changepoint/KDE.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/changepoint/KDE.java @@ -38,7 +38,7 @@ private interface KernelFunction { } private ValueAndMagnitude evaluate(IntervalComplementFunction complement, KernelFunction kernel, double x) { - if (bandwidth == 0.0 || orderedValues.length == 0) { + if (orderedValues.length == 0) { return new ValueAndMagnitude(1.0, 0.0); } int a = Math.min(lowerBound(orderedValues, x - 3.0 * bandwidth), orderedValues.length - 1); diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/aggs/changepoint/ChangePointDetectorTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/aggs/changepoint/ChangePointDetectorTests.java new file mode 100644 index 0000000000000..6fe1108a3f1df --- /dev/null +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/aggs/changepoint/ChangePointDetectorTests.java @@ -0,0 +1,49 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.ml.aggs.changepoint; + +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.xpack.ml.aggs.MlAggsHelper; + +import java.util.Arrays; + +import static org.hamcrest.Matchers.instanceOf; + +public class ChangePointDetectorTests extends ESTestCase { + + /** + * Testing the special case where values without the spike have a variance of 0. + */ + public void testConstantWithSpike() { + double[] values = new double[25]; + Arrays.fill(values, 51); + values[12] = 500001; + + MlAggsHelper.DoubleBucketValues bucketValues = new MlAggsHelper.DoubleBucketValues(null, values); + + ChangeType change = ChangePointDetector.getChangeType(bucketValues); + + assertEquals(12, change.changePoint()); + assertThat(change, instanceOf(ChangeType.Spike.class)); + } + + public void testRandomWithSpike() { + double[] values = new double[25]; + for (int i = 0; i < values.length; i++) { + values[i] = randomLongBetween(50, 53); + } + values[12] = 500001; + + MlAggsHelper.DoubleBucketValues bucketValues = new MlAggsHelper.DoubleBucketValues(null, values); + + ChangeType change = ChangePointDetector.getChangeType(bucketValues); + + assertEquals(12, change.changePoint()); + assertThat(change, instanceOf(ChangeType.Spike.class)); + } +} From 0ad3192abc7b1199d2cffebe39f37f61fa8d16ef Mon Sep 17 00:00:00 2001 From: Jason Whitmore Date: Sun, 12 Oct 2025 15:19:13 -0700 Subject: [PATCH 2/2] Add special case to KDE evaluate() for bandwidth == 0 This change enables an early return and avoids numerical issues. --- .../java/org/elasticsearch/xpack/ml/aggs/changepoint/KDE.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/changepoint/KDE.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/changepoint/KDE.java index e0ac89a7efd35..551d2db2e9580 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/changepoint/KDE.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/changepoint/KDE.java @@ -41,6 +41,10 @@ private ValueAndMagnitude evaluate(IntervalComplementFunction complement, Kernel if (orderedValues.length == 0) { return new ValueAndMagnitude(1.0, 0.0); } + if (bandwidth == 0) { + return new ValueAndMagnitude(x != orderedValues[0] ? 0.0 : 1.0, Math.abs(orderedValues[0] - x)); + } + int a = Math.min(lowerBound(orderedValues, x - 3.0 * bandwidth), orderedValues.length - 1); int b = Math.max(lowerBound(orderedValues, x + 3.0 * bandwidth), a + 1); // Account for all the values outside the interval [a, b) using the kernel complement.