-
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 <olivier.grisel@ensta.org>
Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
|
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 <jeremiedbb@users.noreply.github.com> Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>
Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>
Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>
This was mentionned by one of Oliviers' review but I forgot it. Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
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 <thomasjpfan@gmail.com>
|
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