Skip to content

FIX make deterministic RandomizedSearchCV example#14227

Merged
qinhanmin2014 merged 2 commits intoscikit-learn:masterfrom
glemaitre:is/determinitic_grid_search
Jul 1, 2019
Merged

FIX make deterministic RandomizedSearchCV example#14227
qinhanmin2014 merged 2 commits intoscikit-learn:masterfrom
glemaitre:is/determinitic_grid_search

Conversation

@glemaitre
Copy link
Copy Markdown
Member

It seems that the CI on the PR did not catch that the example was not deterministic.

@glemaitre
Copy link
Copy Markdown
Member Author

@rth Do you think this is the right fix?

>>> from scipy.stats import uniform
>>> iris = load_iris()
>>> logistic = LogisticRegression(solver='saga', tol=1e-2, max_iter=200)
>>> np.random.seed(seed=233423) # make sure uniform will be deterministic
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That would be global I think? Shouldn't we pass a random state to LogisticRegression in addition to the GridSearchCV?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We could but I am not sure that this is actually the issue. I think it should be linked to the uniform for which the random state cannot be set

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Uhm locally, I get determimistic results with master. I see that the behaviour of the random state could be an issue but this is for SciPy < 0.16 which is not the case with the failing build.

@glemaitre
Copy link
Copy Markdown
Member Author

I checked the code under and ParameterGrid will call uniform.rvs(..., random_state=...) so the error should have been trigger with the LogisticRegression then.

@rth could you merge it?

@qinhanmin2014 qinhanmin2014 merged commit 27ad68d into scikit-learn:master Jul 1, 2019
koenvandevelde pushed a commit to koenvandevelde/scikit-learn that referenced this pull request Jul 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants