ENH `learning_curve` raises a warning on failure of CV folds by rprkh · Pull Request #26299 · scikit-learn/scikit-learn

rprkh · 2023-04-29T15:00:45Z

Reference Issues/PRs

Fixes #22057

What does this implement/fix? Explain your changes.

Raises the required warning when some of the folds fail for learning_curve

Any other comments?

thomasjpfan

Thank you for the PR!

thomasjpfan · 2023-05-19T00:43:29Z

+@pytest.mark.parametrize("error_score", [np.nan])
+def test_learning_curve_some_failing_fits_warning(error_score, global_random_seed):


For this specific test, I do not think we need the global_random_seed.

Similarly, since there is only one error_score, we do not need to parametrize it.

thomasjpfan · 2023-05-19T00:46:57Z

+        train_sizes, train_scores, test_scores = learning_curve(
+            svc, X, y, cv=10, error_score=error_score
+        )


There is no need to store

Suggested change

train_sizes, train_scores, test_scores = learning_curve(

svc, X, y, cv=10, error_score=error_score

)

learning_curve(svc, X, y, cv=10, error_score=np.nan)

thomasjpfan · 2023-05-19T00:48:26Z

+- |Enhancement| :func:`model_selection.learning_curve` raises a warning on
+  failure of the `cv` folds. :pr:`26299` by :user:`Rahil Parikh <rprkh>`.


Suggested change

- |Enhancement| :func:`model_selection.learning_curve` raises a warning on

failure of the `cv` folds. :pr:`26299` by :user:`Rahil Parikh <rprkh>`.

- |Enhancement| :func:`model_selection.learning_curve` raises a warning when

every cross validation fold fails. :pr:`26299` by :user:`Rahil Parikh <rprkh>`.

rprkh · 2023-06-02T12:17:46Z

Added the changes @thomasjpfan

thomasjpfan

Thank you for the update! LGTM

github-actions · 2023-11-16T15:51:06Z

✔️ Linting Passed

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

_{Generated for commit: 429fefa. Link to the linter CI: here}

glemaitre

This look good. We can just improve slightly the test.

glemaitre · 2023-11-16T18:17:08Z

+
+    X, y = make_classification(n_classes=3, n_informative=6, shuffle=False)
+    svc = SVC()
+    svc.fit(X, y)


Suggested change

svc.fit(X, y)

glemaitre · 2023-11-16T18:18:36Z

+    warning_message = re.compile(
+        (
+            "10 fits failed out of a total of 50.+The score on these train-test"
+            " partitions for these parameters will be set to nan.+If these failures"
+            " are not expected, you can try to debug them by setting"
+            " error_score='raise'.+ValueError: The number of classes has to be greater"
+            " than one; got 1 class"
+        ),
+        flags=re.DOTALL,
+    )


This would be enough to partially match the warning message.

Suggested change

warning_message = re.compile(

(

"10 fits failed out of a total of 50.+The score on these train-test"

" partitions for these parameters will be set to nan.+If these failures"

" are not expected, you can try to debug them by setting"

" error_score='raise'.+ValueError: The number of classes has to be greater"

" than one; got 1 class"

),

flags=re.DOTALL,

)

warning_message = "10 fits failed out of a total of 50"

glemaitre · 2023-11-16T18:29:11Z

+    with pytest.warns(FitFailedWarning, match=warning_message):
+        learning_curve(svc, X, y, cv=10, error_score=np.nan)


We should also assert for checking that we have the expected np.nan

Suggested change

with pytest.warns(FitFailedWarning, match=warning_message):

learning_curve(svc, X, y, cv=10, error_score=np.nan)

with pytest.warns(FitFailedWarning, match=warning_message):

_, train_score, test_score, *_ = learning_curve(svc, X, y, cv=10, error_score=np.nan)

# the first split should lead to raise the warning

assert np.isnan(train_score[0]).all()

assert np.isnan(test_score[0]).all()

for idx in range(1, train_score.shape[0]):

assert not np.isnan(train_score[idx]).any()

assert not np.isnan(test_score[idx]).any()

rprkh · 2023-11-17T05:26:44Z

Included the suggestions @glemaitre

glemaitre

LGTM

glemaitre · 2023-11-17T10:23:14Z

Thanks @rprkh Merging.

learning_curve raises warnings on failure of folds

e5b2a73

github-actions Bot added the module:model_selection label Apr 29, 2023

lint

5e1ec7f

rprkh changed the title ~~learning_curve raises warnings on failure of folds~~ ENH learning_curve raises warnings on failure of folds Apr 29, 2023

rprkh added 2 commits April 29, 2023 20:54

ilnt

f1f5ece

pass lint

8ab4126

rprkh changed the title ~~ENH learning_curve raises warnings on failure of folds~~ ENH learning_curve raises a warning on failure of CV folds Apr 29, 2023

rprkh added 2 commits April 29, 2023 21:09

remove redundant placeholders

b1c44ad

changelog entry

ce97dfa

thomasjpfan reviewed May 19, 2023

View reviewed changes

rprkh and others added 5 commits June 2, 2023 16:42

include changes

2223f95

Merge branch 'main' into enh_learning_curve

6fb1491

merge conflict line

c55f6f6

lint

3762689

lint

8edc3ce

thomasjpfan approved these changes Jun 2, 2023

View reviewed changes

thomasjpfan added the Waiting for Second Reviewer First reviewer is done, need a second one! label Jun 2, 2023

glemaitre self-requested a review November 16, 2023 15:47

glemaitre added 2 commits November 16, 2023 16:48

Merge remote-tracking branch 'origin' into pr/rprkh/26299

24dd3bf

update changelog

9b1d876

glemaitre reviewed Nov 16, 2023

View reviewed changes

rprkh added 2 commits November 17, 2023 09:51

update test + lint

35209bf

lint

429fefa

glemaitre approved these changes Nov 17, 2023

View reviewed changes

glemaitre merged commit 4081170 into scikit-learn:main Nov 17, 2023

rprkh deleted the enh_learning_curve branch November 17, 2023 15:57

		@pytest.mark.parametrize("error_score", [np.nan])
		def test_learning_curve_some_failing_fits_warning(error_score, global_random_seed):

		- \|Enhancement\| :func:`model_selection.learning_curve` raises a warning on
		failure of the `cv` folds. :pr:`26299` by :user:`Rahil Parikh <rprkh>`.

		with pytest.warns(FitFailedWarning, match=warning_message):
		learning_curve(svc, X, y, cv=10, error_score=np.nan)

-    with pytest.warns(FitFailedWarning, match=warning_message):
-        learning_curve(svc, X, y, cv=10, error_score=np.nan)
+    with pytest.warns(FitFailedWarning, match=warning_message):
+        _, train_score, test_score, *_ = learning_curve(svc, X, y, cv=10, error_score=np.nan)
+    # the first split should lead to raise the warning
+    assert np.isnan(train_score[0]).all()
+    assert np.isnan(test_score[0]).all()
+    for idx in range(1, train_score.shape[0]):
+        assert not np.isnan(train_score[idx]).any()
+        assert not np.isnan(test_score[idx]).any()

Conversation

rprkh commented Apr 29, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reference Issues/PRs

What does this implement/fix? Explain your changes.

Any other comments?

Uh oh!

thomasjpfan left a comment

Choose a reason for hiding this comment

Uh oh!

thomasjpfan May 19, 2023

Choose a reason for hiding this comment

Uh oh!

thomasjpfan May 19, 2023

Choose a reason for hiding this comment

Uh oh!

thomasjpfan May 19, 2023

Choose a reason for hiding this comment

Uh oh!

rprkh commented Jun 2, 2023

Uh oh!

thomasjpfan left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Nov 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✔️ Linting Passed

Uh oh!

glemaitre left a comment

Choose a reason for hiding this comment

Uh oh!

glemaitre Nov 16, 2023

Choose a reason for hiding this comment

Uh oh!

glemaitre Nov 16, 2023

Choose a reason for hiding this comment

Uh oh!

glemaitre Nov 16, 2023

Choose a reason for hiding this comment

Uh oh!

rprkh commented Nov 17, 2023

Uh oh!

glemaitre left a comment

Choose a reason for hiding this comment

Uh oh!

glemaitre commented Nov 17, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rprkh commented Apr 29, 2023 •

edited

Loading

github-actions Bot commented Nov 16, 2023 •

edited

Loading