Skip to content

Use random state to initialize MLPClassifier.#12892

Merged
qinhanmin2014 merged 11 commits intoscikit-learn:masterfrom
xhan7279:test_predict_proba_random_seed_fix
Jan 27, 2019
Merged

Use random state to initialize MLPClassifier.#12892
qinhanmin2014 merged 11 commits intoscikit-learn:masterfrom
xhan7279:test_predict_proba_random_seed_fix

Conversation

@xhan7279
Copy link
Copy Markdown
Contributor

@xhan7279 xhan7279 commented Dec 30, 2018

Reference Issues/PRs

Fixes #12762

What does this implement/fix? Explain your changes.

The test failed if the random seed is changed. Initialize MLP Classifier with a random state seems to resolve the issue.

Any other comments?

NicolasHug
NicolasHug reviewed Dec 30, 2018
View reviewed changes
NicolasHug
NicolasHug reviewed Jan 13, 2019
View reviewed changes
jnothman
jnothman reviewed Jan 14, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@jnothman jnothman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We still ordinarily use a fixed random_state in tests

rth
rth reviewed Jan 19, 2019
View reviewed changes
sklearn/neural_network/tests/test_mlp.py Outdated
with ignore_warnings(category=ConvergenceWarning):
clf.fit(X, y)
clf = MLPClassifier(hidden_layer_sizes=5, activation='logistic'
random_stat=1)
Copy link
Copy Markdown
Member

@rth rth Jan 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a typo in random_state and a comma missing before this parameter.

@jnothman
Copy link
Copy Markdown
Member

jnothman commented Jan 21, 2019

Please merge updated master to avoid the circle ci failure. Let us know if you need help with that. Thanks

@qinhanmin2014
Copy link
Copy Markdown
Member

qinhanmin2014 commented Jan 26, 2019

Hmm, I'm still getting ConvergenceWarning?

System:
python: 3.6.3 |Anaconda custom (64-bit)| (default, Oct 15 2017, 03:27:45) [MSC v.1900 64 bit (AMD64)]
executable: D:\ProgramData\Anaconda3\python.exe
machine: Windows-10-10.0.17134-SP0

BLAS:
macros: SCIPY_MKL_H=None, HAVE_CBLAS=None
lib_dirs: D:\ProgramData\Anaconda3\Library\lib
cblas_libs: mkl_rt

Python deps:
pip: 18.1
setuptools: 40.5.0
sklearn: 0.21.dev0
numpy: 1.15.4
scipy: 1.2.0
Cython: 0.29.2
pandas: 0.23.4

@qinhanmin2014
Copy link
Copy Markdown
Member

qinhanmin2014 commented Jan 26, 2019

I think it's reasonable to keep the ConvergenceWarning, will mere when green.

@qinhanmin2014 qinhanmin2014 merged commit 3207278 into scikit-learn:master Jan 27, 2019
@qinhanmin2014
Copy link
Copy Markdown
Member

qinhanmin2014 commented Jan 27, 2019

thanks @xhan7279

@xhan7279
Copy link
Copy Markdown
Contributor Author

xhan7279 commented Jan 27, 2019

thanks everyone for helping me out =)

@xhan7279 xhan7279 deleted the test_predict_proba_random_seed_fix branch January 27, 2019 15:30
glemaitre pushed a commit to glemaitre/scikit-learn that referenced this pull request Jan 30, 2019
@xhan7279 @glemaitre
TST Use random state to initialize MLPClassifier. (scikit-learn#12892)
f38c8d1
thomasjpfan pushed a commit to thomasjpfan/scikit-learn that referenced this pull request Feb 6, 2019
@xhan7279 @thomasjpfan
TST Use random state to initialize MLPClassifier. (scikit-learn#12892)
1c4b3ab
thomasjpfan pushed a commit to thomasjpfan/scikit-learn that referenced this pull request Feb 7, 2019
@xhan7279 @thomasjpfan
TST Use random state to initialize MLPClassifier. (scikit-learn#12892)
c702658
xhluca pushed a commit to xhluca/scikit-learn that referenced this pull request Apr 28, 2019
@xhan7279
TST Use random state to initialize MLPClassifier. (scikit-learn#12892)
4653a48
xhluca pushed a commit to xhluca/scikit-learn that referenced this pull request Apr 28, 2019
Revert "TST Use random state to initialize MLPClassifier. (scikit-lea…
cf8a999 
…rn#12892)"

This reverts commit 4653a48.
xhluca pushed a commit to xhluca/scikit-learn that referenced this pull request Apr 28, 2019
Revert "TST Use random state to initialize MLPClassifier. (scikit-lea…
e719eb1 
…rn#12892)"

This reverts commit 4653a48.
koenvandevelde pushed a commit to koenvandevelde/scikit-learn that referenced this pull request Jul 12, 2019
@xhan7279 @koenvandevelde
TST Use random state to initialize MLPClassifier. (scikit-learn#12892)
790e3c9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jnothman jnothman jnothman approved these changes

+3 more reviewers

@rth rth rth left review comments

@NicolasHug NicolasHug NicolasHug left review comments

@qinhanmin2014 qinhanmin2014 qinhanmin2014 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

TST: test_predict_proba_binary fails for some random seed

5 participants

@xhan7279 @jnothman @qinhanmin2014 @rth @NicolasHug