ENH Check for nonfinite in mlp models #22150

chritter · 2022-01-08T16:12:31Z

Reference Issues/PRs

Addresses #17925.

What does this implement/fix? Explain your changes.

This implements check for non-finite values of estimator parameters: as produced from optimizers in MLP models.

Any other comments?

Work done with @norbusan.

Originally part of larger PR #22092

thomasjpfan

Thank you for the PR @chritter !

sklearn/neural_network/tests/test_mlp.py

sklearn/neural_network/_multilayer_perceptron.py

thomasjpfan · 2022-01-09T02:21:12Z

sklearn/neural_network/_multilayer_perceptron.py

+                for coef, inter in zip(self.coefs_, self.intercepts_)
+            ]
+        ):
+            raise ValueError("Solver produced non-finite parameter weights.")


Similar comment from https://github.com/scikit-learn/scikit-learn/pull/22149/files#r780722680 We can adjust the message so users know what they can do about it.

Suggested change

raise ValueError("Solver produced non-finite parameter weights.")

raise ValueError("coefs or intercepts have non-finite parameter weights. "

"The input data may contain large values and need to be "

"preprocessed.")

May you include the second part of the message?

May you include the second part of the error message?

Co-authored-by: Norbert Preining <norbert@preining.info>

test_nonfinite_params test data now generated; test parameter in mlp with itertools Co-authored-by: Norbert Preining <norbert@preining.info>

chritter · 2022-01-15T02:26:14Z

Great suggestions @thomasjpfan which I adopted. Thank you!

thomasjpfan

Thanks for the update!

thomasjpfan · 2022-01-15T15:30:09Z

doc/whats_new/v1.1.rst

+:mod:`sklearn.neural_network`
+.............................


Nit: May we move this header up such that the modules are in alphabetical order?

thomasjpfan · 2022-01-15T15:30:09Z

sklearn/neural_network/_multilayer_perceptron.py

+                for coef, inter in zip(self.coefs_, self.intercepts_)
+            ]
+        ):
+            raise ValueError("Solver produced non-finite parameter weights.")


May you include the second part of the message?

thomasjpfan

Thanks for the update!

thomasjpfan · 2022-01-15T16:29:25Z

sklearn/neural_network/_multilayer_perceptron.py

+                for coef, inter in zip(self.coefs_, self.intercepts_)
+            ]
+        ):
+            raise ValueError("Solver produced non-finite parameter weights.")


May you include the second part of the error message?

thomasjpfan

I left a minor comment, otherwise LGTM!

thomasjpfan · 2022-01-15T19:04:24Z

doc/whats_new/v1.1.rst

@@ -462,6 +470,7 @@ Changelog
  left corner of the HTML representation to show how the elements are
  clickable. :pr:`21298` by `Thomas Fan`_.

+


This change is unrelated. Can you revert?

@Thomas9292 This change entry was not introduced in this PR, so I believe no change is needed. Could you clarify?

Note that

- |Enhancement| :func:`utils.estimator_html_repr` displays an arrow on the top left corner of the HTML representation to show how the elements are clickable. :pr:`21298` by `Thomas Fan`_.

is a unique entry.

There is an extra newline added here.

Maybe it's the GitHub interface. I am referring to this newline added by this PR:

Thanks for the clarification.

jjerphan

LGTM after addressing @thomasjpfan's comments.

doc/whats_new/v1.1.rst

sklearn/neural_network/tests/test_mlp.py

improved changelog description Co-authored-by: Julien Jerphanion <git@jjerphan.xyz>

chritter · 2022-01-19T01:35:32Z

@jjerphan Thank you for the review and for the suggestions. Adopted.

* non-finite check in mlp params. Co-authored-by: Norbert Preining <norbert@preining.info> * added changelog entry * updated changelog * simplied nonfinite param check and test; test_nonfinite_params test data now generated; test parameter in mlp with itertools Co-authored-by: Norbert Preining <norbert@preining.info> * reformatting test file * update changelog formatting to pass circleci check * update changelog location alphabetic, extended error message and test * removed empty line in whatsnew * Apply suggestions from code review improved changelog description Co-authored-by: Julien Jerphanion <git@jjerphan.xyz> Co-authored-by: Norbert Preining <norbert@preining.info> Co-authored-by: Julien Jerphanion <git@jjerphan.xyz>

github-actions bot added the module:neural_network label Jan 8, 2022

chritter mentioned this pull request Jan 8, 2022

[MRG] check for non-finite values of estimator parameters #22092

Closed

chritter changed the title ~~test for nonfinite in mlp models~~ [MRG] test for nonfinite in mlp models Jan 8, 2022

chritter force-pushed the issue17925-checkfinite-mlp branch from 7727878 to d9fc9ca Compare January 8, 2022 19:24

thomasjpfan reviewed Jan 9, 2022

View changes

non-finite check in mlp params.

408f5b7

Co-authored-by: Norbert Preining <norbert@preining.info>

norbusan force-pushed the issue17925-checkfinite-mlp branch from d9fc9ca to 3f4c5e1 Compare January 14, 2022 04:55

chritter and others added 3 commits January 14, 2022 13:55

added changelog entry

c3f3060

updated changelog

3f4c5e1

simplied nonfinite param check and test;

1900975

test_nonfinite_params test data now generated; test parameter in mlp with itertools Co-authored-by: Norbert Preining <norbert@preining.info>

chritter added 2 commits January 14, 2022 21:28

reformatting test file

1827299

update changelog formatting to pass circleci check

8396bcf

thomasjpfan reviewed Jan 15, 2022

View changes

thomasjpfan changed the title ~~[MRG] test for nonfinite in mlp models~~ ENH Check for nonfinite in mlp models Jan 15, 2022

thomasjpfan reviewed Jan 15, 2022

View changes

update changelog location alphabetic, extended error message and test

14afdd1

thomasjpfan approved these changes Jan 15, 2022

View changes

thomasjpfan added the Quick Review For PRs that are quick to review label Jan 15, 2022

jjerphan approved these changes Jan 18, 2022

View changes

doc/whats_new/v1.1.rst Outdated Show resolved Hide resolved

sklearn/neural_network/tests/test_mlp.py Outdated Show resolved Hide resolved

jjerphan removed the Quick Review For PRs that are quick to review label Jan 18, 2022

chritter and others added 2 commits January 18, 2022 20:32

removed empty line in whatsnew

eb1745e

Apply suggestions from code review

02e1348

improved changelog description Co-authored-by: Julien Jerphanion <git@jjerphan.xyz>

jjerphan merged commit 9816b35 into scikit-learn:main Jan 19, 2022
29 checks passed

cmarmo mentioned this pull request Aug 15, 2022

Check for NANs and INFs in estimators at the end of fit #17925

Closed

ENH Check for nonfinite in mlp models #22150

ENH Check for nonfinite in mlp models #22150

chritter commented Jan 8, 2022 •

edited

thomasjpfan left a comment

thomasjpfan Jan 9, 2022

thomasjpfan Jan 15, 2022

thomasjpfan Jan 15, 2022

chritter commented Jan 15, 2022

thomasjpfan left a comment

thomasjpfan Jan 15, 2022

thomasjpfan Jan 15, 2022

thomasjpfan left a comment

thomasjpfan Jan 15, 2022

thomasjpfan left a comment

thomasjpfan Jan 15, 2022

chritter Jan 17, 2022 •

edited

thomasjpfan Jan 17, 2022 •

edited

thomasjpfan Jan 17, 2022

chritter Jan 19, 2022

jjerphan left a comment

chritter commented Jan 19, 2022

-            raise ValueError("Solver produced non-finite parameter weights.")
+            raise ValueError("coefs or intercepts have non-finite parameter weights. "
+                             "The input data may contain large values and need to be "
+                             "preprocessed.")

		@@ -462,6 +470,7 @@ Changelog
		left corner of the HTML representation to show how the elements are
		clickable. :pr:`21298` by `Thomas Fan`_.

		:mod:`sklearn.neural_network`
		.............................

ENH Check for nonfinite in mlp models #22150

ENH Check for nonfinite in mlp models #22150

Conversation

chritter commented Jan 8, 2022 • edited

Reference Issues/PRs

What does this implement/fix? Explain your changes.

Any other comments?

thomasjpfan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chritter commented Jan 15, 2022

thomasjpfan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thomasjpfan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thomasjpfan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chritter Jan 17, 2022 • edited

Choose a reason for hiding this comment

thomasjpfan Jan 17, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jjerphan left a comment

Choose a reason for hiding this comment

chritter commented Jan 19, 2022

chritter commented Jan 8, 2022 •

edited

chritter Jan 17, 2022 •

edited

thomasjpfan Jan 17, 2022 •

edited