This is an automated email from the ASF dual-hosted git repository.

jihao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git


The following commit(s) were added to refs/heads/master by this push:
     new 632f48b  [TE] detection -  zero divide fix (#4018)
632f48b is described below

commit 632f48bbc012892db92fe831f23042f934c079e5
Author: Jihao Zhang <jihzh...@linkedin.com>
AuthorDate: Tue Mar 26 10:58:46 2019 -0700

    [TE] detection -  zero divide fix (#4018)
    
    Fix 0.0 divide
    Refactor anomaly baseline & current value filling
---
 .../detection/components/PercentageChangeRuleDetector.java  |  8 ++++----
 .../detection/wrapper/BaselineFillingMergeWrapper.java      | 13 ++++++++-----
 .../components/PercentageChangeRuleDetectorTest.java        |  2 +-
 3 files changed, 13 insertions(+), 10 deletions(-)

diff --git 
a/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/components/PercentageChangeRuleDetector.java
 
b/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/components/PercentageChangeRuleDetector.java
index 59fe1aa..ec828c6 100644
--- 
a/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/components/PercentageChangeRuleDetector.java
+++ 
b/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/components/PercentageChangeRuleDetector.java
@@ -90,11 +90,11 @@ public class PercentageChangeRuleDetector implements 
AnomalyDetector<PercentageC
 
     // calculate percentage change
     df.addSeries(COL_CHANGE, map((Series.DoubleFunction) values -> {
-      if (values[1] == 0) {
-        return values[0] == 0 ? 0.0 : (values[0] > 0 ? 
Double.POSITIVE_INFINITY : Double.NEGATIVE_INFINITY);
+      if (Double.compare(values[1], 0.0) == 0) {
+        return Double.compare(values[0], 0.0) == 0 ? 0.0 : (values[0] > 0 ? 
Double.POSITIVE_INFINITY : Double.NEGATIVE_INFINITY);
       }
-      return values[0] / values[1];
-    }, df.getDoubles(COL_CURR), df.get(COL_BASE)).subtract(1));
+      return (values[0] - values[1]) / values[1];
+    }, df.getDoubles(COL_CURR), df.get(COL_BASE)));
 
     // defaults
     df.addSeries(COL_ANOMALY, BooleanSeries.fillValues(df.size(), false));
diff --git 
a/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/wrapper/BaselineFillingMergeWrapper.java
 
b/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/wrapper/BaselineFillingMergeWrapper.java
index e07cf83..8355b5c 100644
--- 
a/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/wrapper/BaselineFillingMergeWrapper.java
+++ 
b/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/wrapper/BaselineFillingMergeWrapper.java
@@ -22,6 +22,7 @@ package org.apache.pinot.thirdeye.detection.wrapper;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.Collections2;
 import java.util.HashMap;
+import java.util.stream.Collectors;
 import org.apache.pinot.thirdeye.dataframe.DoubleSeries;
 import org.apache.pinot.thirdeye.dataframe.Series;
 import org.apache.pinot.thirdeye.dataframe.util.MetricSlice;
@@ -124,7 +125,13 @@ public class BaselineFillingMergeWrapper extends 
MergeWrapper {
 
   @Override
   protected List<MergedAnomalyResultDTO> 
merge(Collection<MergedAnomalyResultDTO> anomalies) {
-    return this.fillCurrentAndBaselineValue(super.merge(anomalies));
+    List<MergedAnomalyResultDTO> mergedAnomalies = super.merge(anomalies);
+
+    // skip current & baseline filling if the anomaly is not merged
+    this.fillCurrentAndBaselineValue(mergedAnomalies.stream()
+        .filter(mergedAnomaly -> !isExistingAnomaly(this.existingAnomalies, 
mergedAnomaly)).collect(Collectors.toList()));
+
+    return mergedAnomalies;
   }
 
   @Override
@@ -157,10 +164,6 @@ public class BaselineFillingMergeWrapper extends 
MergeWrapper {
    */
   List<MergedAnomalyResultDTO> 
fillCurrentAndBaselineValue(List<MergedAnomalyResultDTO> mergedAnomalies) {
     for (MergedAnomalyResultDTO anomaly : mergedAnomalies) {
-      // skip current & baseline filling if the anomaly is not merged
-      if (this.isExistingAnomaly(this.existingAnomalies, anomaly)) {
-        continue;
-      }
       try {
         String metricUrn = anomaly.getMetricUrn();
         MetricEntity me = MetricEntity.fromURN(metricUrn);
diff --git 
a/thirdeye/thirdeye-pinot/src/test/java/org/apache/pinot/thirdeye/detection/components/PercentageChangeRuleDetectorTest.java
 
b/thirdeye/thirdeye-pinot/src/test/java/org/apache/pinot/thirdeye/detection/components/PercentageChangeRuleDetectorTest.java
index b63c50b..7ad66d9 100644
--- 
a/thirdeye/thirdeye-pinot/src/test/java/org/apache/pinot/thirdeye/detection/components/PercentageChangeRuleDetectorTest.java
+++ 
b/thirdeye/thirdeye-pinot/src/test/java/org/apache/pinot/thirdeye/detection/components/PercentageChangeRuleDetectorTest.java
@@ -182,7 +182,7 @@ public class PercentageChangeRuleDetectorTest {
     percentageRule.init(spec, new DefaultInputDataFetcher(this.provider, -1));
     List<MergedAnomalyResultDTO> anomalies = percentageRule.runDetection(new 
Interval(1551398400000L, 1551571200000L), "thirdeye:metric:1");
     Assert.assertEquals(anomalies.size(), 1);
-    Assert.assertEquals(anomalies.get(0).getStartTime(), 1551398400000L);
+    Assert.assertEquals(anomalies.get(0).getStartTime(), 1551484800000L);
     Assert.assertEquals(anomalies.get(0).getEndTime(), 1551488400000L);
 
   }


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org

Reply via email to