ENH Move threadpoolctl outside of iteration loop in KMeans

[jeremiedbb]

Related to #17208

Profiling shows that the threadpoolctl context manager has a significant overhead when running KMeans on very small datasets. This PR moves the call to threadpoolctl outside of the iteration loop.

The timings on my laptop from the snippet in #17208 are now:
0.22: 0.0138
0.23: 0.0562
master: 0.0431
this pr: 0.0214

Although it's a nice improvement, we don't fully recover the perf of 0.22. See explanation and discussion in #17208.

[jeremiedbb]

ping @adrinjalali

[jeremiedbb]

The uncovered lines are for the verbose mode.

[NicolasHug]

Can you help me understand what made you change your mind about https://github.com/scikit-learn/scikit-learn/pull/16499/files#r382599621 @jeremiedbb ?

[NicolasHug]

Although it's a nice improvement, we don't fully recover the perf of 0.22. See explanation and discussion in #17208.

Just trying to fully understand where we're at: is this sentence still up to date considering that the overhead of _deprecate_positional_args has been ruled out?

[jeremiedbb]

Can you help me understand what made you change your mind about https://github.com/scikit-learn/scikit-learn/pull/16499/files#r382599621 @jeremiedbb ?

Haha I was sure you'd remember :D
I did not benchmark very small datasets at that time and the timings reported in #17208 indicate that the overhead is significant at that scale.

Reading my comment again, I recall what was the issue.

for i in range(max_iter):
    elkan_iter(...)
    ...
    euclidean_distances(...)  # BLAS

If we put the context inside the loop we have the overhead and if we put it outside of the loop, the inner euclidean_distances is limited. I don't see how we can solve this :/
Let me put back the wip tag...

[jeremiedbb]

Just trying to fully understand where we're at: is this sentence still up to date considering that the overhead of _deprecate_positional_args has been ruled out?

We still don't exactly recover the timings from 0.22 but the explanation is wrong, we still need to figure out why

[jeremiedbb]

actually, there's no need for the threadpoolctl context for elkan since elkan does not call BLAS in nested parallel regions. I just removed it. The timings are now:

	elkan	lloyd
0.22	0.0138	0.0211
master	0.0477	0.0428
this pr	0.0134	0.0181

[NicolasHug]

I just removed it

These are the best fixes

LGTM as long as the answer to my question below is yes ;)

thanks @jeremiedbb

[NicolasHug]

Ok so you confirm that we have no BLAS call that are protected by this CM, apart from the ones in lloyd_iter?

[jeremiedbb]

Yes I just double checked

[NicolasHug]

also will need a whatsnew entry

[jeremiedbb]

@NicolasHug I merged the what's new entry with the entry for #17210 if you don't mind. They are 2 attempts at fixing the same thing.

[thomasjpfan]

As another datapoint:

0.22: 0.0229
0.23: 0.16056
master: 0.1285
this pr: 0.0249

LGTM Thank you @jeremiedbb !