Repository navigation
Distributed srm - #220
Distributed srm#220
Conversation
| The estimated noise variance :math:`\\rho_i^2` for each subject | ||
|
|
||
| comm_ : mpi4py.MPI.Intracomm | ||
| The MPI communicator containing the data |
There was a problem hiding this comment.
comm should be used instead of comm_ everywhere, because it is given as a parameter, not computed from data:
http://scikit-learn.org/stable/developers/contributing.html#instantiation
http://scikit-learn.org/stable/developers/contributing.html#estimated-attributes
|
|
||
| for subject in range(subjects): | ||
| mu.append(np.mean(data[subject], 1)) | ||
| np.random.seed(subject) |
There was a problem hiding this comment.
How does this interact with self.rand_seed?
There was a problem hiding this comment.
Check issue #114
Is it right to initialize the distributed SRM randomly?
There was a problem hiding this comment.
I removed this random seed per subject code. Instead, each SRM class instantiation can set a random seed (as before). This means for multiple ranks, each rank can potentially set a different seed. I don't think we can guarantee exactly deterministic results for the distributed implementation due to differences in parallel scheduling per run.
| print("Test: different number of samples per subject") | ||
|
|
||
|
|
||
| test_can_instantiate() |
There was a problem hiding this comment.
Wrap this call in if __name__ == "__main__", please.
|
Jenkins, retest this please. |
1 similar comment
|
Jenkins, retest this please. |
|
@mjanderson09 great job! Don't need to do it now. You can do it in a different PR. Thank you! |
| s.fit(X) | ||
| sr_v0_4 = np.load('sr_v0_4.npy') | ||
| diff_v0_4 = np.linalg.norm(sr_v0_4 - s.s_) | ||
| assert(diff_v0_4 < 1e-5) |
There was a problem hiding this comment.
Shouldn't the results be element-wise equal, i.e., np.allclose(sr_v0_4, s.s_), maybe with an atol value?
|
|
||
| # Check that runs with 2 subject | ||
| s.fit(X) | ||
| sr_v0_4 = np.load('sr_v0_4.npy') |
There was a problem hiding this comment.
Add this to imports:
from pathlib import Path
Then:
Path(__file__).parent / "sr_v0_4.npy"
|
@TuKo, what say you? |
|
@TuKo says NIPS, sorry... |
| print("Test: different number of samples per subject") | ||
|
|
||
|
|
||
| test_can_instantiate() |
There was a problem hiding this comment.
Please guard this call with if __name__ == "__main__".
|
👍 Great work! |
|
For the record, here is the effect of the changes on the accuracy in the image prediction example: |
#220) * Added test for retriving variables from an optimizer * Added comments to test * Addressed comments * Fixed travis bug * Added fix to circular controls * Added set for explored operations and duplicate prefix stripping * Removed embeded ipython * Removed prefix, use seperate graph for each network * Removed redundant imports * Addressed comments and added separate graph to initializer * fix typos * get rid of prefix in documentation
No description provided.