CLAassistant
commented
5 years ago
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.
This is a renewed pull request for the 'millisecond' precision fixes.
I added a fix for the delta_sigma bug to the previous branch but it appears somebody else beat me to it, here: https://github.com/twitter/AnomalyDetection/pull/59 . I don't have anything to add to this, so in order to not pollute the branch I'm leaving it out.
This pull request contains two commits, each one dealing with the fact that the data we're using uses ms precision, but contains measurements that come in every 5 minutes - from a set of monitoring stations, so they come in in bunches.
Granularity detection assumes that if you have 'X' precision your data also contains one measurement per X, but that is clearly not always the case, as the above testcase demonstrates. Worse, the code doesn't even deal with the possibility that the granularity detection might say "ms". The switch in question simply doesn't handle it, causing the code to blow up with an error. Ditto for seconds.
The basic "it blows up" thing is easily fixed, but the deeper issue of handling measurement intervals that are disparate from the measurement's precision is not. That requires a rethink of the code.