Skip to content

[ML] Testing for no change should not use the change trend model #1415

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 17, 2020
Merged

[ML] Testing for no change should not use the change trend model #1415

merged 1 commit into from
Jul 17, 2020

Conversation

@tveasey
Copy link
Contributor

@tveasey tveasey commented Jul 17, 2020

Change #1391 introduced a subtle error: when computing the likelihood of no change we should use a reference to the current time series trend model, not the copy of the model which assumes a change is occurring. This has not yet been released so I've marked this as a non-issue.

@tveasey tveasey requested a review from droberts195 July 17, 2020 06:48
droberts195
droberts195 approved these changes Jul 17, 2020
View reviewed changes
Copy link

@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
Copy link
Contributor Author

tveasey commented Jul 17, 2020

We'd like this to make 7.8.1. Linux failed to download the docker image and we don't support aarch yet so I'm going to go ahead and merge this.

@tveasey tveasey merged commit c69873f into elastic:master Jul 17, 2020
@tveasey tveasey deleted the changepoint-bug branch July 17, 2020 07:47
This was referenced Jul 17, 2020
Merged
Merged
Merged
Merged
tveasey added a commit that referenced this pull request Jul 17, 2020
@tveasey
[ML] Testing for no change should not use the change trend model (#1416)
d0d3f60 
Change #1391 introduced a subtle error: when computing the likelihood of no change
we should use a reference to the current time series trend model, not the copy of the
model which assumes a change is occurring.

Backport #1415.
@droberts195 droberts195 added v7.8.1 and removed v7.8.2 labels Jul 17, 2020
tveasey added a commit that referenced this pull request Jul 17, 2020
@tveasey
[ML] Testing for no change should not use the change trend model (#1418)
44edb1d 
Change #1391 introduced a subtle error: when computing the likelihood of no change
we should use a reference to the current time series trend model, not the copy of the
model which assumes a change is occurring.

Backport #1415.
tveasey added a commit that referenced this pull request Jul 20, 2020
@tveasey
[ML] Testing for no change should not use the change trend model (#1419)
fcd3a56 
Change #1391 introduced a subtle error: when computing the likelihood of no change
we should use a reference to the current time series trend model, not the copy of the
model which assumes a change is occurring.

Backport #1415.
tveasey added a commit that referenced this pull request Jul 20, 2020
@tveasey
[ML] Testing for no change should not use the change trend model (#1417)
d1d99b7 
Change #1391 introduced a subtle error: when computing the likelihood of no change
we should use a reference to the current time series trend model, not the copy of the
model which assumes a change is occurring.

Backport #1415.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@droberts195 droberts195 droberts195 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

>bug >non-issue v6.8.12 v7.8.1 v7.9.0 v7.10.0 v8.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@tveasey @droberts195