-
-
Notifications
You must be signed in to change notification settings - Fork 8.7k
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
Quick fix for memory leak in CPU Hist. #5153
Conversation
Closes dmlc#3579 .
Looking back, this PR is only made possible by many previous refactorings. |
@hcho3 @CodingCat Is there any particular bug for hist that's considered as critical? We @RAMitchell would like to make |
Ping @hcho3 |
@trivialfis I’m okay with making |
@hcho3 Well, improvement is endless unless there's a better algorithm succeeds Are you Okay with this PR? |
@trivialfis I was referring to future improvements. |
@hcho3 Got it. Will create a new PR for setting it as default. Would you like to review this fix? |
Closes #3579 .
The proper fix is putting quantiles in
DMatrix
, which we have already done for GPU Hist. See #5143 for related future roadmap. This PR is a band-aid for fixing the old issue as we want a fully working 1.0 release.