TST Relax `test_minibatch_sensible_reassign` to avoid CI failures with single global random seed #29278

lesteve · 2024-06-17T11:56:29Z

Summarising my comments below, relaxing the check to be > 9 instead of > 10 makes the CI pass on all random seeds, see Azure logs on PR commit 162684a. I have not been able to reproduce the issue locally.

This issue has been seen in multiple CI builds from time to time e.g. #27967 (comment) or #26802 (comment)

github-actions · 2024-06-17T11:57:47Z

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

_{Generated for commit: 3abcd1c. Link to the linter CI: here}

lesteve · 2024-06-17T15:05:54Z

Some observations

for one build there is the info about the number of clusters which is right on the boundary so using > 9 may be a quick fix

E       AssertionError: assert 10 > 10
E        +  where 10 = <built-in method sum of numpy.ndarray object at 0x7fda357b5090>()
E        +    where <built-in method sum of numpy.ndarray object at 0x7fda357b5090> = array([ True, False,  True,  True, False, False, False,  True,  True,\n        True, False, False,  True, False, False, False, False,  True,\n        True,  True]).sum
E        +      where array([ True, False,  True,  True, False, False, False,  True,  True,\n        True, False, False,  True, False, False, False, False,  True,\n        True,  True]) = <built-in method any of numpy.ndarray object at 0x7fda488d9e10>(axis=1)
E        +        where <built-in method any of numpy.ndarray object at 0x7fda488d9e10> = array([[ -0.08411287,  -1.35341192],\n       [  0.        ,   0.        ],\n       [-10.04085907,  10.04083876],\n       ...     ],\n       [ -9.1038364 ,   6.86739558],\n       [ -9.66539459,   5.01011406],\n       [ -6.894048  ,  -1.75933572]]).any
E        +          where array([[ -0.08411287,  -1.35341192],\n       [  0.        ,   0.        ],\n       [-10.04085907,  10.04083876],\n       ...     ],\n       [ -9.1038364 ,   6.86739558],\n       [ -9.66539459,   5.01011406],\n       [ -6.894048  ,  -1.75933572]]) = MiniBatchKMeans(batch_size=10, init='random', n_clusters=20, random_state=34).cluster_centers_

for some builds sometimes it passes sometimes it does not, for example Ubuntu_Jammy_Jellyfish pymin_conda_forge_openblas_ubuntu_2204, Haswell passes build log, SkylakeX fails build log

lesteve · 2024-06-17T16:25:12Z

For all the failing builds the number of clusters is 10, so using > 9 may be a quick fix:

ValueError: Number of non-zero clusters is too small num_non_zero_clusters=10

Linux_Runs pylatest_conda_forge_mkl Details
Linux_free_threaded pylatest_pip_free_threaded Details
Ubuntu_Jammy_Jellyfish pymin_conda_forge_openblas_ubuntu_2204 Details

test_minibatch_sensible_reassign

lesteve · 2024-06-18T04:28:10Z

I tested all the random seeds in the CI and the only problematic seed on all four failing builds is global_random_seed=34 with num_non_zero_clusters=10, see Azure logs

free-threaded fails with Haskell (still can't reproduce locally though with same CPU architecture ...), the two other failing builds using OpenBLAS are using SkyLakeX, the MKL one we don't know (sklearn.show_versions() does not give the CPU architecture info)

test_minibatch_sensible_reassign

lesteve · 2024-06-18T05:13:54Z

Relaxing the number of clusters check to be > 9 rather than > 10, all random seed CI pass see Azure logs, I am going to trigger the normal CI and make this as ready for review.

lesteve · 2024-06-18T05:24:16Z

sklearn/cluster/tests/test_k_means.py

@@ -437,21 +437,24 @@ def test_minibatch_sensible_reassign(global_random_seed):
        n_clusters=20, batch_size=10, random_state=global_random_seed, init="random"
    ).fit(zeroed_X)
    # there should not be too many exact zero cluster centers
-    assert km.cluster_centers_.any(axis=1).sum() > 10
+    num_non_zero_clusters = km.cluster_centers_.any(axis=1).sum()
+    assert num_non_zero_clusters > 9, f"{num_non_zero_clusters=} is too small"


I added assertion string (i.e. second argument in the assert, not sure if there is a more exact name) because it seems like pytest assertion rewriting is a bit broken (needs a bit of investigation as to why). If this ever fails again, at least the message will tell how far we are from the threshold

jeremiedbb

LGTM. Ideally the test should be reworked with assertion where we know in advance what we really expect, but let's go with this quick tweak for now.

…h single global random seed (scikit-learn#29278)

…h single global random seed (#29278)

lesteve added 2 commits June 17, 2024 13:55

NOMRG Try to trigger test_minibatch_sensible_reassign

6e9d4f9

[free-threaded]

5648219

github-actions bot added the module:cluster label Jun 17, 2024

lesteve marked this pull request as draft June 17, 2024 11:56

[free-threaded] trigger CI

c0cab37

lesteve mentioned this pull request Jun 17, 2024

⚠️ CI failed on Linux_free_threaded.pylatest_pip_free_threaded (last failure: Jun 14, 2024) ⚠️ #29253

Closed

[free-threaded] warnings as error should do the trick right? right?

6df6157

marenwestermann mentioned this pull request Jun 17, 2024

TST use global_random_seed in sklearn/decomposition/tests/test_factor_analysis.py #29272

Merged

lesteve added 2 commits June 17, 2024 17:07

[free-threaded] use error rather than assertion

c0a9a6a

[free-threaded] oh well ...

b141137

lesteve added 2 commits June 18, 2024 06:09

[free-threaded] [all random seeds]

7d4e1b3

test_minibatch_sensible_reassign

[free-threaded] [azure parallel] [all random seeds]

c09298b

test_minibatch_sensible_reassign

lesteve added 2 commits June 18, 2024 06:28

[free-threaded] relax constraint [all random seeds]

38fc24c

test_minibatch_sensible_reassign

[free-threaded] go back to assertions [all random seeds]

162684a

test_minibatch_sensible_reassign

lesteve marked this pull request as ready for review June 18, 2024 05:13

[free-threaded] [azure parallel] trigger normal CI

3abcd1c

lesteve changed the title ~~NOMRG Test minibatch sensible reassign~~ TST Relax test_minibatch_sensible_reassign to avoid CI failures with single global random seed Jun 18, 2024

github-actions bot added the Build / CI label Jun 18, 2024

lesteve commented Jun 18, 2024

View reviewed changes

lesteve added No Changelog Needed Quick Review For PRs that are quick to review labels Jun 18, 2024

jeremiedbb approved these changes Jun 20, 2024

View reviewed changes

jeremiedbb merged commit a4f4efe into scikit-learn:main Jun 20, 2024
41 of 47 checks passed

lesteve deleted the test_minibatch_sensible_reassign branch June 20, 2024 12:57

lesteve mentioned this pull request Jun 20, 2024

⚠️ CI failed on Linux.pylatest_pip_openblas_pandas ⚠️ #27967

Closed

lesteve mentioned this pull request Jun 20, 2024

⚠️ CI failed on Linux_Runs.pylatest_conda_forge_mkl (last failure: Jul 10, 2024) ⚠️ #26802

Closed

jeremiedbb pushed a commit to jeremiedbb/scikit-learn that referenced this pull request Jul 2, 2024

TST Relax test_minibatch_sensible_reassign to avoid CI failures wit…

8eae334

…h single global random seed (scikit-learn#29278)

jeremiedbb mentioned this pull request Jul 2, 2024

Release 1.5.1 #29382

Merged

11 tasks

jeremiedbb pushed a commit to jeremiedbb/scikit-learn that referenced this pull request Jul 2, 2024

TST Relax test_minibatch_sensible_reassign to avoid CI failures wit…

66bc8c4

…h single global random seed (scikit-learn#29278)

jeremiedbb pushed a commit that referenced this pull request Jul 2, 2024

TST Relax test_minibatch_sensible_reassign to avoid CI failures wit…

ded9890

…h single global random seed (#29278)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TST Relax `test_minibatch_sensible_reassign` to avoid CI failures with single global random seed #29278

TST Relax `test_minibatch_sensible_reassign` to avoid CI failures with single global random seed #29278

lesteve commented Jun 17, 2024 •

edited

Loading

github-actions bot commented Jun 17, 2024 •

edited

Loading

lesteve commented Jun 17, 2024 •

edited

Loading

lesteve commented Jun 17, 2024 •

edited

Loading

lesteve commented Jun 18, 2024 •

edited

Loading

lesteve commented Jun 18, 2024

lesteve Jun 18, 2024 •

edited

Loading

jeremiedbb left a comment

TST Relax test_minibatch_sensible_reassign to avoid CI failures with single global random seed #29278

TST Relax test_minibatch_sensible_reassign to avoid CI failures with single global random seed #29278

Conversation

lesteve commented Jun 17, 2024 • edited Loading

github-actions bot commented Jun 17, 2024 • edited Loading

✔️ Linting Passed

lesteve commented Jun 17, 2024 • edited Loading

lesteve commented Jun 17, 2024 • edited Loading

lesteve commented Jun 18, 2024 • edited Loading

lesteve commented Jun 18, 2024

lesteve Jun 18, 2024 • edited Loading

Choose a reason for hiding this comment

jeremiedbb left a comment

Choose a reason for hiding this comment

TST Relax `test_minibatch_sensible_reassign` to avoid CI failures with single global random seed #29278

TST Relax `test_minibatch_sensible_reassign` to avoid CI failures with single global random seed #29278

lesteve commented Jun 17, 2024 •

edited

Loading

github-actions bot commented Jun 17, 2024 •

edited

Loading

lesteve commented Jun 17, 2024 •

edited

Loading

lesteve commented Jun 17, 2024 •

edited

Loading

lesteve commented Jun 18, 2024 •

edited

Loading

lesteve Jun 18, 2024 •

edited

Loading