Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ML] Ensure we have sufficient resolution when we reinitialise component models #2167

Merged
merged 5 commits into from Jan 6, 2022

Conversation

tveasey
Copy link
Contributor

@tveasey tveasey commented Jan 6, 2022

When we retest if a modelled seasonal component is still appropriate or may contain bias we now require higher resolution if shorter time windows are available. This is important because if the time series has a mixture of fast and slow seasonality we will detect the fast seasonality first and then potentially reinitialise it when we detect the slow seasonality. We should only do this if the window provides us with sufficient resolution to initialise it properly.

Closes #2166.

@droberts195
Copy link
Contributor

In the case of 30 minute periodicity being detected first, then 24 hour periodicity being detected later, is the effect of this PR that we'll opt not to model the 24 hour periodicity at all if doing so would ruin the modelling of the 30 minute periodic pattern?

@tveasey
Copy link
Contributor Author

tveasey commented Jan 6, 2022

In the case of 30 minute periodicity being detected first, then 24 hour periodicity being detected later, is the effect of this PR that we'll opt not to model the 24 hour periodicity at all if doing so would ruin the modelling of the 30 minute periodic pattern?

No. We simply are more careful about which windows will trigger us to reinitialise components. In particular, we will only consider reinitialising a component if we have sufficient resolution. There is a tradeoff here since the current models will contain bias due to averaging over the unmodelled components. Either way it'll rectify, but I think it probably makes sense to keep the current model if we can't do a good job of reinitialising it on the principle of least surprise.

Here are the results in a prototypical example, from the unit test I added, with blue actual and red model predictions (note the second one is more zoomed but they both cover the detection of the daily periodicity, it just isn't as clear in the second case because there is no reinitialisation issue).

before

Screenshot 2022-01-06 at 12 13 26

after

Screenshot 2022-01-06 at 12 10 26

Copy link
Contributor

@droberts195 droberts195 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tveasey tveasey removed the WIP label Jan 6, 2022
@tveasey tveasey merged commit f9cd002 into elastic:main Jan 6, 2022
@tveasey tveasey deleted the issue-2166 branch January 6, 2022 19:05
tveasey added a commit to tveasey/ml-cpp-1 that referenced this pull request Jan 6, 2022
…ent models (elastic#2167)

When we retest if a modelled seasonal component is still appropriate or may contain bias we now require higher
resolution if shorter time windows are available. This is important because if the time series has a mixture of fast and
slow seasonality we will detect the fast seasonality first and then potentially reinitialise it when we detect the slow
seasonality. We should only do this if the window provides us with sufficient resolution to initialise it properly.

Closes elastic#2166.
tveasey added a commit to tveasey/ml-cpp-1 that referenced this pull request Jan 6, 2022
…ent models (elastic#2167)

When we retest if a modelled seasonal component is still appropriate or may contain bias we now require higher
resolution if shorter time windows are available. This is important because if the time series has a mixture of fast and
slow seasonality we will detect the fast seasonality first and then potentially reinitialise it when we detect the slow
seasonality. We should only do this if the window provides us with sufficient resolution to initialise it properly.

Closes elastic#2166.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ML] Better handling of edge cases when reinitialising models after detecting new seasonal components
2 participants