-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
FEA Support missing-values in ExtraTrees*
#28268
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
Conversation
Signed-off-by: Adam Li <adam2392@gmail.com>
Signed-off-by: Adam Li <adam2392@gmail.com>
Signed-off-by: Adam Li <adam2392@gmail.com>
Signed-off-by: Adam Li <adam2392@gmail.com>
Signed-off-by: Adam Li <adam2392@gmail.com>
Signed-off-by: Adam Li <adam2392@gmail.com>
… into extratreenan
Signed-off-by: Adam Li <adam2392@gmail.com>
… into extratreenan
Signed-off-by: Adam Li <adam2392@gmail.com>
Signed-off-by: Adam Li <adam2392@gmail.com>
Signed-off-by: Adam Li <adam2392@gmail.com>
Signed-off-by: Adam Li <adam2392@gmail.com>
Signed-off-by: Adam Li <adam2392@gmail.com>
Signed-off-by: Adam Li <adam2392@gmail.com>
Signed-off-by: Adam Li <adam2392@gmail.com>
… into extratreenan
… into extratreenan
Signed-off-by: Adam Li <adam2392@gmail.com>
…into extraforest
Signed-off-by: Adam Li <adam2392@gmail.com>
Signed-off-by: Adam Li <adam2392@gmail.com>
Signed-off-by: Adam Li <adam2392@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks @adam2392. Would it be suitable for ExtraTreesRegressor to be added to test_non_supported_criterion_raises_error_with_missing_values
?
Co-authored-by: Omar Salman <omar.salman2007@gmail.com>
Co-authored-by: Omar Salman <omar.salman2007@gmail.com>
Signed-off-by: Adam Li <adam2392@gmail.com>
…into extraforest
Good point @OmarManzoor! Added in 1441925 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Otherwise, LGTM
Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>
Reference Issues/PRs
Follow-up to: #27966
Closes: #27931
What does this implement/fix? Explain your changes.
ExtraTrees*
Any other comments?
This should be a very easy review as this just activates the usage of missing value support and adds unit-test parameterizations for the ExtraTree Forest. Just requesting reviews from @thomasjpfan, @glemaitre and @OmarManzoor since you all were able to review the code in #27966. Hope this is easy :)
Waiting for #27966 to be merged first.This just shows the relevant code changes that introduces the missing-value support overall forExtraTrees
. Note the bulk of the work for going from tree->forest missing-value support was added in #26391. The main change here is to introduce certain unit tests.ExtraTrees*
perform much better on data with missing values compared to naive imputation onmain
ExtraTree*
compared to naive imputation onmain
Benchmarks on ExtraTrees demonstrating there is no runtime regression