-
-
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
Cleanup CPU predict function. #11139
Conversation
7a9c90f
to
d40949e
Compare
@razdoburdin Could you please help take a look into the optimization? I'm not an expert in CPU optimization. The changes in the predictor affects the Xeon much more significantly than the Ryzen. If I remove the dense optimization, it adds about 3 seconds to Ryzen, but 20 seconds to the Xeon. Looking at some profiling results on Ryzen, the bottleneck seems to be in data loading ( |
It is hard to give the exact answer without deep investigation of the changes. My hypothesis are:
|
@razdoburdin Thank you for sharing, could you please help review the changes in the CPU predictor when you are available?
Currently, the evaluation might be even more expensive than training for some datasets. Would be great if we could get some help on that. |
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.
The PR looks good for me.
As for future prediction optimization, I plan to work on it, but latter this year.
Thank you for looking into it. Feel free to ping me if there's anything I can help. |
@hcho3 Could you please help approve the PR if there's no further change request? The CI failure is unrelated (sklearn update). |
Partially address #10793
The optimization mostly focuses on dense data and the result varies between CPUs: