Skip to content

Commit

Permalink
fixing conf band creation in prohpet predict function
Browse files Browse the repository at this point in the history
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
  • Loading branch information
irumata authored and facebook-github-bot committed Dec 5, 2024
1 parent 8a79280 commit a8f7a48
Showing 1 changed file with 4 additions and 1 deletion.
5 changes: 4 additions & 1 deletion kats/detectors/prophet_detector.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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(
Expand Down

0 comments on commit a8f7a48

Please sign in to comment.