-
Notifications
You must be signed in to change notification settings - Fork 9
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
Anomaly detection using Isolation Forest #41
Anomaly detection using Isolation Forest #41
Conversation
Signed-off-by: Shashank Reddy Boyapally <sboyapal@redhat.com>
Signed-off-by: Shashank Reddy Boyapally <sboyapal@redhat.com>
Signed-off-by: Shashank Reddy Boyapally <sboyapal@redhat.com>
Signed-off-by: Shashank Reddy Boyapally <sboyapal@redhat.com>
Signed-off-by: Shashank Reddy Boyapally <sboyapal@redhat.com>
Signed-off-by: Shashank Reddy Boyapally <sboyapal@redhat.com>
"config": config_file_name, | ||
"output_path": "output.csv", | ||
"hunter_analyze": True, | ||
"output_format": "json", | ||
"anomaly_detection": False, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While we already have hunter_analyze
to indicate that this is related to change point detection, do we need anamaly_detection
as an argument here? Vice versa applies to the endpoint /daemon/anomaly
too.
|
||
|
||
def run(**kwargs): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shashank-boyapally Is there a specific reason we have replace **kwargs with the OptionMap? OptionMap doesn't seem to be required. We can achieve thread safety in both if that is our primary concern.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason why we are going ahead with OptionMap structure instead of kwargs is, some of the options are being used across other functions such as the anomaly window for anomaly detection. For cleaner code it was better to call OptionMap instead of passing kwargs throughout.
dataframe_json = self.dataframe.to_json(orient="records") | ||
dataframe_json = json.loads(dataframe_json) | ||
|
||
for index, entry in enumerate(dataframe_json): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was just thinking if we can abstract out these structure that give us only a list of changepoints and the full list of json. As they can reused by any model integration. @shashank-boyapally any thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will be adding this to in follow-on PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we convert this comment into an RFE issue? So we don't lose track
@shashank-boyapally nice addition. Can we think of a better directory structure by grouping things into multiple packages like having all the algos at one place and the rest categorized as well? That way we won't end up having everything in the root directory. |
Could we introduce a new structure in a follow on PR @vishnuchalla ? |
lgtm! Tested locally. Address any concerns @vishnuchalla has. |
Signed-off-by: Shashank Reddy Boyapally <sboyapal@redhat.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Type of change
Description
Support for anomaly detection added to orion, this allows the ability to add other algorithms as well. Below are the outputs of the anomaly detection run on small scale cdv2. It is to be noted that the deviation in percentage change reported is the change in percentage from the weighted mean over a window at that particular datapoint
Similarly in daemon mode one can now reach out to the endpoint
daemon/anomaly
below is the sample output of a json listIt is to be noted that the endpoint to changepoint is now moved to
daemon/changepoint
from/daemon
Related Tickets & Documents
Checklist before requesting a review
Testing