Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable interpolation in HCAD cold start #575

Merged
merged 3 commits into from
Jun 22, 2022

Conversation

kaituo
Copy link
Collaborator

@kaituo kaituo commented Jun 20, 2022

Description

Previously, we used interpolation in HCAD cold start for the purpose of efficiency. This caused problems for model accuracy. This PR removes interpolation in the cold start step.

Testing done:

  1. added unit tests to verify precision boosted.

Signed-off-by: Kaituo Li [email protected]

Issues Resolved

#539

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Previously, we used interpolation in HCAD cold start for the purpose of efficiency. This caused problems for model accuracy. This PR removes interpolation in the cold start step.

Testing done:
1. added unit tests to verify precision boosted.

Signed-off-by: Kaituo Li <[email protected]>
@codecov-commenter
Copy link

codecov-commenter commented Jun 20, 2022

Codecov Report

Merging #575 (bbf9809) into main (7f3820a) will decrease coverage by 0.03%.
The diff coverage is 91.66%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main     #575      +/-   ##
============================================
- Coverage     79.03%   79.00%   -0.04%     
- Complexity     4203     4204       +1     
============================================
  Files           296      296              
  Lines         17679    17682       +3     
  Branches       1879     1880       +1     
============================================
- Hits          13973    13969       -4     
- Misses         2806     2813       +7     
  Partials        900      900              
Flag Coverage Δ
plugin 79.00% <91.66%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
.../java/org/opensearch/ad/AnomalyDetectorPlugin.java 96.53% <ø> (ø)
...pensearch/ad/settings/AnomalyDetectorSettings.java 100.00% <ø> (ø)
...n/java/org/opensearch/ad/ml/EntityColdStarter.java 82.00% <91.66%> (+0.21%) ⬆️
...rch/ad/transport/ForwardADTaskTransportAction.java 94.06% <0.00%> (-3.39%) ⬇️
...java/org/opensearch/ad/task/ADBatchTaskRunner.java 81.76% <0.00%> (-0.76%) ⬇️
...rch/ad/transport/AnomalyResultTransportAction.java 80.13% <0.00%> (-0.69%) ⬇️
...ain/java/org/opensearch/ad/task/ADTaskManager.java 76.67% <0.00%> (+0.15%) ⬆️
...opensearch/ad/indices/AnomalyDetectionIndices.java 72.12% <0.00%> (+0.18%) ⬆️
...ain/java/org/opensearch/ad/model/ModelProfile.java 72.72% <0.00%> (+3.63%) ⬆️

@kaituo kaituo requested a review from amitgalitz June 20, 2022 22:07
Signed-off-by: Kaituo Li <[email protected]>
amitgalitz
amitgalitz previously approved these changes Jun 20, 2022
Copy link
Member

@amitgalitz amitgalitz left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the change

@@ -258,6 +258,8 @@ private AnomalyDetectorSettings() {}

public static final int MAX_IMPUTATION_NEIGHBOR_DISTANCE = 2;

public static final boolean ALLOW_INTERPLATION_IN_COLDSTART = false;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we make this as dynamic setting?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

changed

@kaituo kaituo merged commit 03e04d7 into opensearch-project:main Jun 22, 2022
opensearch-trigger-bot bot pushed a commit that referenced this pull request Jun 22, 2022
* Disable interpolation in HCAD cold start

Previously, we used interpolation in HCAD cold start for the purpose of efficiency. This caused problems for model accuracy. This PR removes interpolation in the cold start step.

Testing done:
1. added unit tests to verify precision boosted.

Signed-off-by: Kaituo Li <[email protected]>
(cherry picked from commit 03e04d7)
kaituo added a commit that referenced this pull request Jun 22, 2022
* Disable interpolation in HCAD cold start

Previously, we used interpolation in HCAD cold start for the purpose of efficiency. This caused problems for model accuracy. This PR removes interpolation in the cold start step.

Testing done:
1. added unit tests to verify precision boosted.

Signed-off-by: Kaituo Li <[email protected]>
(cherry picked from commit 03e04d7)

Co-authored-by: Kaituo Li <[email protected]>
@amitgalitz amitgalitz added the enhancement New feature or request label Jun 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants