Skip to content

[ML] Don't lose mean offset in expanding window when persisting #99

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

Merged
merged 2 commits into from
May 23, 2018

Conversation

tveasey
Copy link
Contributor

@tveasey tveasey commented May 22, 2018

We were not persisting and restoring the mean offset time of the values added to the expanding window w.r.t. the start of the data buckets. This was therefore getting reset to 0 on a persist restore cycle.

This will introduce a small error when initialising periodic components after a persist and restore and would affect results in this context.

Release note: Fixes persist and restore, which were missing some of the trend model state.

@tveasey tveasey force-pushed the bug/persist-and-restore-mean-offset branch from e05cbd0 to 1770b6f Compare May 22, 2018 11:16
Copy link
Contributor

@edsavage edsavage 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 merged commit a6e65b6 into elastic:master May 23, 2018
tveasey added a commit to tveasey/ml-cpp-1 that referenced this pull request May 25, 2018
…tic#99)

We were not persisting and restoring the mean offset time of the values added to the 
expanding window w.r.t. the start of the data buckets. This was therefore getting reset to 0 on 
a persist restore cycle. This will introduce a small error when initialising periodic components 
after a persist and restore.
@sophiec20 sophiec20 added :ml and removed :ml labels Jun 12, 2018
@tveasey tveasey deleted the bug/persist-and-restore-mean-offset branch December 13, 2018 08:40
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.

3 participants