[ML] Improve time series decomposition in the presence of change points #198
Conversation
…on-with-change-points
…on-with-change-points
…on-with-change-points
…on-with-change-points
| times.push_back(time); | ||
| trend.push_back(weight * daily); | ||
| } | ||
| for (auto offset : {0 * DAY, 5 * DAY}) { |
There was a problem hiding this comment.
This is just the same test performed for different offsets of the start time: the bulk of the changes are indentation.
|
|
||
| stats.s_V0 = CBasicStatistics::variance(moments); | ||
| stats.s_DF0 = b + static_cast<double>(stats.s_Segmentation.size() - 2); | ||
| } else { |
There was a problem hiding this comment.
The old function was this branch.
| if (stats.s_Segmentation.size() > 0) { | ||
| buckets = CTimeSeriesSegmentation::removePiecewiseLinearScaledPeriodic( | ||
| buckets, stats.s_Segmentation, stats.s_T0[0], stats.s_Scales); | ||
| } else { |
There was a problem hiding this comment.
The old function was this branch.
| return this->testVariance(window, values, period_, df1, v1, stats, R, | ||
| meanRepeats, pVariance) || | ||
| this->testAmplitude(window, values, period_, b, v, R, meanRepeats, | ||
| pVariance, stats); |
There was a problem hiding this comment.
I factored out testVariance and testAmplitude from this function (for reuse in CPeriodicityHypothesisTests::testPeriodWithScaling). These are what is being compared to the following code.
…on-with-change-points
|
|
||
| private: | ||
| using THypothesisVec = std::vector<CNestedHypotheses>; | ||
|
|
||
| private: | ||
| //! The test. | ||
| TTestFunc m_Test; | ||
| //! True if the hypothesis used a piecewise linear trend. | ||
| std::size_t m_TrendSegments; |
There was a problem hiding this comment.
I'm not sure that the comments here quite match the type of the member variable
There was a problem hiding this comment.
Agreed. This used to be a bool but we actually need the number of segments in the calling code so this changed. I'll update.
| @@ -281,7 +286,7 @@ void project(const TFloatMeanAccumulatorVec& values, | |||
| core_t::TTime bucketLength, | |||
| TFloatMeanAccumulatorVec& result) { | |||
| result.clear(); | |||
| if (!values.empty()) { | |||
| if (values.size() > 0) { | |||
There was a problem hiding this comment.
I'm curious about the change here (and elsewhere)... why not prefer empty over checking size?
There was a problem hiding this comment.
Our standards now say to not use ! but instead == false to match Elastic Java style. I personally find size() > 0 easier to read than empty() == false at a glance. I changed to be more compliant with our style guidelines because I was making quite a few changes in this file.
| double B{static_cast<double>(std::count_if( | ||
| // Compute the number of non-empty buckets. This needs to account for | ||
| // projecting onto windows. | ||
| stats.s_B = static_cast<double>(std::count_if( |
There was a problem hiding this comment.
It might be useful to take the opportunity to rename the data members of STestStats to something more descriptive?
| rng.generateNormalSamples(0.0, 1.0, trend.size(), noise); | ||
|
|
||
| core_t::TTime startTesting{pad + 28 * HOUR}; | ||
| //TDoubleVec thresholds[]{TDoubleVec{0.08, 0.08}, TDoubleVec{0.18, 0.15}}; |
There was a problem hiding this comment.
nitpick: delete this line rather than comment out?
…on-with-change-points
…on-with-change-points
tveasey
deleted the
enhancement/improve-timeseries-decomposition-with-change-points
branch
The problems related to #159 showed up because of a fundamental issue with the way we decompose a time series in the presence of change points. In particular, we don't consider that the window of values we sketch might include common types of change points, that we've handled in the rest of our modelling since #92. There are two potential issues as a result:
This PR tackles this problem head on by allowing for the common types of change point we handle, linear scaling and piecewise linear trend, when testing for and initialising new components.
The bulk of this change is in code to perform and test top-down segmentation of a window of values,
CTimeSeriesSegmentation, allowing for either piecewise linear trend or piecewise constant modulation of a seasonal component. We use our standard approach of doing model selection based statistical significance of the explained variance to choose the segmentation. (We can do this relatively easily because each candidate partition creates a nested hypothesis.) On this front, although bottom up partitioning is generally considered preferable, this is primarily concerned with dealing with large change events, which will be identified by a top-down approach. It is also much more compute efficient and we do this multiple times for different hypotheses we test to find the best decomposition.