Correct seasonal decomposition

[tailaiw]

As @yy910616 pointed out in #13, our implementation of STL decomposition was wrong and it was basically classic seasonal decomposition (trend as moving average, seasonal as average across periods, plus residual). In this PR, we want to correct it.

• Merge the original NaiveSeasonalAD (a special version of classic decomposition where the trend is preassumed constant 0) with original STLDecomposition (which is in fact classic seasonal decomposition as @yy910616 pointed out), and make it ClassicSeasonalDecomposition
• Implement the real STLDecomposition
• Updated docs

Updated: for item 2, we decided to hold on for statsmodels new release (details in the thread).

[tailaiw]

@yy910616 I have done the easy part. I put a placeholder for the STLDecomposition. One thing I noticed is that the extraction of trend and seasonal in STL decomposition is coupled, therefore it cannot be fit and then predict (on the contrary, classic decomposition may fit the seasonal periodic pattern first, and then reused it in prediction as detrending and deseasoning are two steps). It means that the core algorithm of STLDecomposition should go to _predict_core and the transformer itself does not need fit. Feel free to let me know if you have any thoughts on this.

[yangyu-1]

Hi @tailaiw, the documentations looks really good to me.
I'm still unfamiliar with large parts of your code base, so I have no input there.

I'm not entirely sure if it was necessarily to introduce a breaking change just to rename "Naive" to "Classic", since they seem similar enough to me. But that's a very minor issue. I'll leave that up to you

[yangyu-1]

For Implement the real STLDecomposition, I have a couple of notes. @tailaiw Please let me know what you think:

1. The STLDecom algorithm I'm referring to is in this package by jrmontag.
2. I see what you mean by STLDecom algorithm should go to _predict_core instead of _fit_core. To check my mental model, the STLDecom algorithm first calculates trend with a lowess smoothing process, then calculates seasonality on the detrended data by averaging user-input seasonal frequency. Therefor there can't be a seasonality component without the lowess smoothing first, and hence _fit_core, which is used to extract seasonality, is no longer needed for ADTK's STLDecomposition. This would cause ADTK's STLDecomposition method to diverge from ClassicSeasonalDecomposition, and that's why you broked out the _Deseasonal class to allow STLDecomposition & ClassicSeasonalDecomposition to behave differently. Does that sound about right?
3. Unfortunately, if that is the case, I didn't find any other method in terms of calculating seasonal component prior to / independent of detrending via lowess.
4. I'm not sure how fit_core is used downstream, but if that is a concern, we can maybe use fit_core in STLDecomposition to do both lowess smoothing and calculating seasonality. Basically filling seasonal and trend during the _fit_core call? That way _predict_core can be just for prediction and _fit_core for extraction. Maybe that sounds more logical from a design standpoint, but I'm not sure.

Now for the STL algorithm, I've found some concerning posts about STLComposition:

1. I started with this SO post, which lead me to this discussion.
2. I've tested the STLDecomposition package using the same dataset simple mock dataset. It does seem to have some issue.
   
3. Furthermore, some folks have pointed out that the python package does not fully implement STLDecom by the original paper. I haven't read the paper in depth yet, but it does seem to be true, given that seasonal component is not modeled, but relies on user input. I read more into this, but it looks like seasonality has to be from user input.
4. If we implement this method in ADTK, it'd most likely be LowessSmoothing(Trend) + autocorrelation(Seasonality)? I'm honestly not sure how that'll perform. At the very least, I think calling it STLDecompose could be misleading.

I'm not sure what should be the next step. We could proceed to implement the LowessSmoothing(Trend) + autocorrelation(Seasonality) method, but will have to test it out against some seasonal+trend data. Or we could shelf this method for now, and wait to see if statsmodel's decompose method will incorporate STLDecompose.

Let me know what you think.

[tailaiw]

@yy910616 It is fine if we want to call fit_predict directly because the time series used to fit the model is the same one we want to find anomalies from. What is not doable is calling fit with a training series and then calling predict on the other. Because the way STL extracts trend and seasonal pattern results in the fact that the seasonal pattern depends on the trend (as you mentioned in item 2, and in fact it is an iterative process in the original paper.

We assume that the users of ADTK can be split into two groups. The first is, I have a time series, I want to find anomalies from it. The second is, I have a historical time series (say the temperature of the past few years), I want to train a model with it so that the model may help me with finding anomalies in future (say the temperature in future). For the first user group, they only need fit_predict method. For the second group, they need fit for training (maybe also fit_predict for model evaluation with historical data), and predict for future real-time prediction.

In classic seasonal decomposition, we assume what needs to be learned by the model is the seasonal pattern which will not change over time (from past to future). The trend is something you always need to extract on the fly. Under this assumption, the classic decomposition can serve both user groups. In particular, for the second group, the model learns the seasonal pattern from historical data, and when the model is applied to the new data the seasonal pattern is removed directly as it is irrelevant to the process of extracting trend.

In STL, however, extracting trends in the new data will automatically be bundled with extracting the seasonal pattern. Then the second user group's assumption that seasonal pattern doesn't change over time is violated, and we are unclear what to do. So my proposal is, putting everything of STL in predict and only serve the first user group because there is nothing to "learn" and everything is on the fly. I'm totally okay with that as long as we clearly inform the user in docstrings and documentation. Many other transformers (e.g. RollingAggregate, DoubleRollingAggregate) are all "predict-only" anyway (and that's why they have attribute _need_fit as False).

[tailaiw]

Regarding the STL algorithm implementation, I also noticed the package by @jrmontag. It seems the only reasonably robust package I can find. But I notice that statsmodels is developing their STL and it looks it may come with v0.11 release (hopefully soon?).

One of our core developers happens to know @jrmontag and it seems the statsmodels implementation is expected to be more robust according to him. So what do you think about holding on this issue for now until their release?

[jrmontag]

👋 howdy! just to add a bit more context here, I'd encourage you to prefer the (forthcoming?) statsmodels implementation over the STLDecompose library that I authored. While I don't have any issue, per se, with folks building on top of STLDecompose, it was a bit of a hacky project and I'm confident you'll have better support in the longer term using whatever comes out of the statsmodels authors. Feel free to use any/all/none of STLDecompose as inspiration, but I hope the statsmodels release will have a friendlier API than my version which is more of a "reach inside the internals and tie some pieces together" implementation 😄

[yangyu-1]

Thank you @tailaiw for the detail explanation, and thank you @jrmontag for the input on STLDecompose.

@tailaiw I think it definitely makes sense to wait for the STLDecompose in statsmodels.

[tailaiw]

@jrmontag Thanks for sharing your insights. Looks that matches how we understand.

[tailaiw]

@yy910616 Great. Here is what I will do. I will clean up the code and docs (including merging some new API introduced in the master branch by #19), and only keep the classic decomposition for now. I will remove the placeholder of STL for now. When it is ready, I will request your review. Upon your approval, I will close this PR. But I will keep issue #13 open for future implementation of STLDecomposition (after a robust STL is released in statsmodels). Does that sound good to you?

[yangyu-1]

@tailaiw Thank you so much for checking and working on this. I have no project currently that depends on STL so feel free to make the changes. We've been using the package internally and has been giving us some great insights. Please let me know if you need help anywhere!