From a8f7a48a58d5cefb2d4e33cacc0a9f7033288a11 Mon Sep 17 00:00:00 2001 From: Nick Knize Date: Thu, 5 Dec 2024 08:52:33 -0800 Subject: [PATCH] fixing conf band creation in prohpet predict function Summary: **Current Logic Issue: Incorrect Anomaly Response Behavior** In the current implementation, we are simply populating the reference to predicted time series as the confidence band. This is not the expected behavior for `AnomalyResponse` constructor, as it expects time series to be independent. The issue arises from the fact that changing one anomaly response parameter affects another. Specifically, when we modify the upper bound of the confidence band, it also changes the `predicted_ts` parameter. This leads to incorrect results when calling the "extend" function on an `AnomalyResponse` object created by Prophet's predict method. As a result, we end up extending the `predicted_ts` three times when calling the "extend" function. **Example Use Case:** Suppose we have an `AnomalyResponse` object created by Prophet's predict method, and we want to extend its time series using the "extend" function. With the current logic, this would lead to incorrect results, as the `predicted_ts` parameter would be extended three times. ```python anomaly_response = prophet.predict(...) print(len(anomaly_response.predicted_ts.toList)) # e.g., 10 anomaly_response2 = prophet.predict(...) print(len(anomaly_response2.predicted_ts.toList)) # e.g., 100 anomaly_response.extend(anomaly_response2) # incorrect results print(len(anomaly_response.predicted_ts.toList)) # currently returns 310 ``` **Solution:** To fix this issue, we need to modify the logic to generate the time series independently and avoid affecting other parameters when changing one anomaly response parameter. We can achieve this by creating a copy of the time series instead of referencing the original one. ```python # Create a copy of the time series anomaly_response.confidence_band = anomaly_response.confidence_band.copy() # Now, modifying the upper bound of the confidence band will not affect the predicted_ts parameter anomaly_response.confidence_band.upper = ... ``` By making this change, we ensure that the time series are independent, and modifying one anomaly response parameter does not affect another. Differential Revision: D66768001 fbshipit-source-id: 07f2eff2fe991d1d3480c439966072e73ee5986a --- kats/detectors/prophet_detector.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/kats/detectors/prophet_detector.py b/kats/detectors/prophet_detector.py index 41c6aaae..517c7463 100644 --- a/kats/detectors/prophet_detector.py +++ b/kats/detectors/prophet_detector.py @@ -41,6 +41,7 @@ PROPHET_YHAT_UPPER_COLUMN = "yhat_upper" HOLIDAY_NAMES_COLUMN_NAME = "holiday" HOLIDAY_DATES_COLUMN_NAME = "ds" +import copy import os import sys @@ -562,7 +563,9 @@ def predict( # If not using z-score, set confidence band equal to prediction if model.uncertainty_samples == 0: - confidence_band = ConfidenceBand(upper=predicted_ts, lower=predicted_ts) + confidence_band = ConfidenceBand( + upper=copy.deepcopy(predicted_ts), lower=copy.deepcopy(predicted_ts) + ) else: confidence_band = ConfidenceBand( upper=TimeSeriesData(