-
Notifications
You must be signed in to change notification settings - Fork 135
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
changed behavior of rand_seed in BRSA #239
Conversation
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.
We now have guidelines on using random numbers:
https://github.com/IntelPNI/brainiak/blob/master/CONTRIBUTING.rst#standards
Therefore, all new commits should conform to the standard. Specifically, you should not use numpy.random.random()
; you should use numpy.random.RandomState
.
Hi @mihaic Sorry I am a bit confused. How should numpy.random.RandomState replace numpy.random.random()? I thought the former sets the state while the latter just return a random number. |
Does that mean I should call this |
You do not need to use |
Please see if the new commit is correct. Thanks! |
@@ -686,7 +688,7 @@ def _fit_RSA_UV(self, X, Y, X0, | |||
n_C = np.size(X, axis=1) | |||
l_idx = np.tril_indices(n_C) | |||
|
|||
np.random.seed(self.rand_seed) | |||
self.random_state_ = check_random_state(self.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.
Please document this attribute in the class docstring. Could be something like this:
random_state_: `RandomState`
Random number generator initialized using 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.
Thanks. Just added exactly this text.
Per request by @mshvartsman, rand_seed argument now defaults to None, and BRSA won't reset random seed if None is provided.
The suppression of warning is also removed.