-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
[MRG] Fix parallelisation of kmeans clustering #12955
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
Conversation
Strange that this doctest result changes... the doctest hasn't been changed in 40e6c43 or subsequently :| |
But it's only failing on the latest numpy and scipy... so it's probably due to an upstream change. Can you change the example to make it deterministic? For example, move the top-right point further right? |
I can reproduce the issue and I confirm that this PR fixes it. |
I have updated the doctest to be more deterministic by moving a set of cluster points far apart from the other. |
Please add an entry to the change log for version 0.20.3 at |
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.
thanks @nixphix I agree that we don't need a test here.
please add a what's new entry.
Btw, why do current code use all the cores when n_clusters=1 (and not when n_clusters>1)?
And seems that we have a similar problem
Feel free to fix it in another PR, otherwise we'll open an issue. |
I can't reproduce that. However, it might be that the multithreading comes from MKL in that case. Running the snippet with the env variable |
6e43bfe
to
0fa1263
Compare
Thanks, I don't have time to reproduce but your version seems more reasonable. According to the code, before this PR, there's no parallelism at scikit-learn level. I guess the reporter make some mistakes accidentally. |
Sure will fix it in another PR |
Thanks @nixphix |
This reverts commit f5e9d8e.
This reverts commit f5e9d8e.
Reference Issues/PRs
Fixes #12949
What does this implement/fix? Explain your changes.
Fixes parallelisation of kmean clustering