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

[REVIEW] Deprecating quantile-per-tree and removing three previously deprecated Random Forest parameters #3667

Merged

Conversation

vinaydes
Copy link
Contributor

This PR marks quantile-per-tree parameter of Random Forest for deprecation. Enabling quantile computation per tree has significant performance impact without much accuracy gain. Similar levels of accuracy can be achieved with higher number of bins for global quantiles. Therefore this feature would be removed from subsequent release.

This PR also removes three previously deprecated RF parameters:
seed was deprecated in 0.16 and was planed to be removed in 0.17
min_rows_per_node was deprecated in 0.17 and was planed to be removed in 0.18
rows_sample was deprecated in 0.17 and was planed to be removed 0.18

Also minor correction in documentation.

@vinaydes vinaydes requested a review from a team as a code owner March 29, 2021 05:39
@GPUtester
Copy link
Contributor

Can one of the admins verify this patch?

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@github-actions github-actions bot added the Cython / Python Cython or Python issue label Mar 29, 2021
Copy link
Member

@teju85 teju85 left a comment

Choose a reason for hiding this comment

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

Changes LGTM. However, we'll need approval from @JohnZed or @dantegd for this one.

@vinaydes vinaydes changed the title [WIP] Deprecating quantile-per-tree and removing three previously deprecated Random Forest parameters [REVIEW] Deprecating quantile-per-tree and removing three previously deprecated Random Forest parameters Mar 30, 2021
@teju85 teju85 added breaking Breaking change doc Documentation labels Mar 30, 2021
Copy link
Contributor

@JohnZed JohnZed left a comment

Choose a reason for hiding this comment

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

Nice to get rid of some of those antiques, thanks.

@JohnZed
Copy link
Contributor

JohnZed commented Mar 30, 2021

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 3df57f3 into rapidsai:branch-0.19 Mar 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Breaking change Cython / Python Cython or Python issue doc Documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants