-
Notifications
You must be signed in to change notification settings - Fork 89
Adding Random State #45
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…into random_state
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one comment. otherwise looks good
evalml/tests/test_pipelines.py
Outdated
clf_2.fit(X, y) | ||
|
||
assert clf_1.score(X, y) == clf.score(X, y) | ||
assert clf.score(X, y) != clf_2.score(X, y) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think this might not be a good test since technically we cannot guarantee that the two clfs will perform different just because they have random seeds
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Go ahead and merge!
Made sure random state was set in models, pipelines, and wherever randomness occurs.
Fixes #11.