Skip to content

[ML] Fix potential undefined behaviour in classification and regression training #1997

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
Aug 23, 2021

Conversation

tveasey
Copy link
Contributor

@tveasey tveasey commented Aug 20, 2021

The issue was caused by a mismatch in the condition to decide whether to split the root node and the condition to decide whether to bother to initialise all its state. In particular, we would, if we got very unlucky, try and split having not properly initialised state. This updates the offending condition so we always properly initialise state when we're going to try and split. (There was also a typo this fixes.)

Closes #1956.

@tveasey
Copy link
Contributor Author

tveasey commented Aug 23, 2021

retest

Copy link
Contributor

@przemekwitek przemekwitek 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 b69aa9b into elastic:main Aug 23, 2021
tveasey added a commit to tveasey/ml-cpp-1 that referenced this pull request Aug 23, 2021
…on training (elastic#1997)

The issue was caused by a mismatch in the condition to decide whether to split the root node and the condition to
decide whether to bother to initialise all its state. In particular, we would, if we got very unlucky, try and split having not properly initialised state. This updates the offending condition so we always properly initialise state when we're going
to try and split.

Closes elastic#1956.
tveasey added a commit to tveasey/ml-cpp-1 that referenced this pull request Aug 23, 2021
…on training (elastic#1997)

The issue was caused by a mismatch in the condition to decide whether to split the root node and the condition to
decide whether to bother to initialise all its state. In particular, we would, if we got very unlucky, try and split having not properly initialised state. This updates the offending condition so we always properly initialise state when we're going
to try and split.

Closes elastic#1956.
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] hit a SIGSEGV error for classification
2 participants