Skip to content

Fix 3rdparty detector bugs#45

Open
pmenn36 wants to merge 2 commits intohtm-community:masterfrom
pmenn36:fix-3rdparty-detector-bugs
Open

Fix 3rdparty detector bugs#45
pmenn36 wants to merge 2 commits intohtm-community:masterfrom
pmenn36:fix-3rdparty-detector-bugs

Conversation

@pmenn36
Copy link

@pmenn36 pmenn36 commented Oct 2, 2020

I noticed when evaluating my own detector using this process that there were two bugs that got in the way:

  1. The default NAB labels contained duplicate rows, but the output of my detector did not contain duplicates. This resulted in a length difference between the labels and the anomaly scores, which raised an AssertionError. I fixed this by adding an optional --removeDuplicateLabels flag. The default behavior remains unchanged.
  2. The anomaly scores from my output csv were being read in as strings instead of floats. I forced casting to float in sweeper.py, which does not affect the default behavior because the anomaly scores from the default detectors are correctly read in as floats anyway.

Patrick Menninger added 2 commits October 1, 2020 20:15
1. NAB benchmark contained duplicate timestamps which caused length to not match up with imported results, throwing errors
2. Threshold values were read in as strings instead of floats, throwing errors

Signed-off-by: Patrick Menninger <patrick.menninger@viasat.com>
Duplicate fixes from previous commit broke the default detectors, so I made them optional

Signed-off-by: Patrick Menninger <patrick.menninger@viasat.com>
@ctrl-z-9000-times
Copy link
Collaborator

Hi,

Sorry it took a year for anyone to respond to your PR.
For what its worth, I just now merged your changes for casting the thresholds to floating point numbers.

I didn't merge the stuff about duplicate records BC I've never encountered that bug.
If that is a real bug and something that you still want fixed then ping me (@ctrl-z-9000-times )
and I will review and merge that as well.

Thanks!

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