-
Notifications
You must be signed in to change notification settings - Fork 1
ENH Rework PairwiseDistancesArgKmin to use a class method .compute
#5
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
ENH Rework PairwiseDistancesArgKmin to use a class method .compute
#5
Conversation
Co-authored-by: Olivier Grisel <[email protected]>
Co-authored-by: Olivier Grisel <[email protected]>
|
WDYT, @thomasjpfan? |
This computes the exact number of threads needed (effective threads) at the initialisation. This allows allocating the exact number of pointers that we need. This also allows removing the `num_threads` parameters from the callbacks methods' signatures (as the information is now accessible via an attribute). Co-authored-by: Jérémie du Boisberranger <[email protected]> Co-authored-by: Thomas J. Fan <[email protected]>
Co-authored-by: Thomas J. Fan <[email protected]>
This was mentionned by one of Oliviers' review but I forgot it. Co-authored-by: Olivier Grisel <[email protected]>
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.
I have a few more thoughts, but I think it's time to merge (I can not merge into your fork). It would be good to see how the original PR looks now.
Co-authored-by: Thomas J. Fan <[email protected]>
|
Merging based on @thomasjpfan's last comment: #5 (review) I'll update the plan with this sub-PR reviews' comments that I see there. |
Reference Issues/PRs
Small rework for scikit-learn#21462
What does this implement/fix? Explain your changes.
Addresses discussions in https://github.com/scikit-learn/scikit-learn/pull/21462/files#r744318407
cc @thomasjpfan