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

add thresholded rcf #215

Merged
merged 1 commit into from
Sep 16, 2021
Merged

add thresholded rcf #215

merged 1 commit into from
Sep 16, 2021

Conversation

wnbts
Copy link
Contributor

@wnbts wnbts commented Sep 15, 2021

Description

This pr adds thresholded rcf to replace existing rcf and sketch-based threshold.

Check List

  • Commits are signed per the DCO using --signoff

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.

@wnbts wnbts changed the base branch from main to 1.1 September 15, 2021 20:02
@wnbts wnbts marked this pull request as ready for review September 15, 2021 21:02
@kaituo
Copy link
Collaborator

kaituo commented Sep 15, 2021

Can you create a remote branch and we can open PR to check into code there? Don't want to impact other people's testing. After we are ready, we will check into mainline and and backport to 1.1.

@wnbts wnbts changed the base branch from 1.1 to trcf September 15, 2021 21:10
@wnbts
Copy link
Contributor Author

wnbts commented Sep 15, 2021

yes, i am using the trcf remote branch.

if (json.has(ENTITY_TRCF)) {
trcf = toTrcf(json.getAsJsonPrimitive(ENTITY_TRCF).getAsString());
}
// TODO: convert rcf and threshold to trcf
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you put line 560~571 under the else branch? Want to make it explicit that we only expect trcf or rcf/threshold, but not both.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, updated it.

@wnbts wnbts merged commit b0d8330 into opensearch-project:trcf Sep 16, 2021
kaituo pushed a commit to kaituo/anomaly-detection-1 that referenced this pull request Sep 23, 2021
kaituo pushed a commit to kaituo/anomaly-detection-1 that referenced this pull request Sep 25, 2021
kaituo pushed a commit to kaituo/anomaly-detection-1 that referenced this pull request Sep 25, 2021
kaituo added a commit that referenced this pull request Sep 27, 2021
* [1.1] Bump OpenSearch core to 1.1 in CI (#212)

Signed-off-by: Tyler Ohlsen <[email protected]>

* add thresholded rcf (#215)

Signed-off-by: lai <[email protected]>

* Integrating with ThresholdedRandomCutForest

This PR contains:

PRs (all approved)  related to integrating ThresholdedRandomCutForest:
#221
#222
#223
#224
#226
#227
#228

rebased PR:
#201
#202
#216

* pass the correct shingleSize to ThresholdedRandomCutForest

Previously, I used shingleSize 1 for externally shingled ThresholdedRandomCutForest because of the double multiplication with shingle size in RCF.  Now RCF has fixed the issue. This commits adds new RCF libraries from aws/random-cut-forest-by-aws#278 and passes the correct shingleSize to ThresholdedRandomCutForest.

This commits adds new RCF libraries from aws/random-cut-forest-by-aws#278 and passes the correct shingleSize to ThresholdedRandomCutForest.

Co-authored-by: Tyler Ohlsen <[email protected]>
Co-authored-by: Lai <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants