Skip to content

Conversation

Jason-Whitmore
Copy link
Contributor

Issue #127517 describes ChangePointDetector.getChangeType() failing to detect a spike when all non spike values are constant (for example, the input [51,51,...,51,50001,51...,51]).

Prior to this change, the KDE.evaluate() method would return a special object for an input with bandwidth == 0. This would occur if the values excluding the spike had zero variance (see KDE constructor). This would then cause ChangePointDetector.getChangeType() and SpikeAndDipDetector.detect() to use the wrong p-value and fail to detect the spike.

This change removes the bandwidth == 0 condition for returning a special ValueAndMagnitude object. Statistical testing in ChangePointDetector.getChangeType() and SpikeAndDipDetector.detect() now properly detect the spike in the example mentioned above.

Unit tests are added to confirm the bug is fixed.

Closes #127517

…ct 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.
@elasticsearchmachine elasticsearchmachine added needs:triage Requires assignment of a team area label v9.1.0 external-contributor Pull request authored by a developer outside the Elasticsearch team labels May 29, 2025
@nielsbauman nielsbauman added >bug :ml Machine learning Team:ML Meta label for the ML team and removed needs:triage Requires assignment of a team area label labels Jun 4, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/ml-core (Team:ML)

@davidkyle
Copy link
Member

@elasticmachine test this please

@davidkyle davidkyle self-assigned this Jun 10, 2025
@davidkyle
Copy link
Member

Thanks for your contribution @Jason-Whitmore and thanks for adding the tests, we will try to review this as soon as possible

@davidkyle
Copy link
Member

@elasticmachine test this please

Copy link
Contributor

@tveasey tveasey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I propose a small tweak.

This change enables an early return and avoids numerical issues.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>bug external-contributor Pull request authored by a developer outside the Elasticsearch team :ml Machine learning Team:ML Meta label for the ML team v9.3.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ChangePointDetector doesn't detect a spike

5 participants