srm: subtract the subject-specific mean in srm.transform() and srm.transform_subject()#495
srm: subtract the subject-specific mean in srm.transform() and srm.transform_subject()#495qihongl wants to merge 6 commits into
Conversation
|
Hi @mihaic I made a new PR about SRM intercept here. |
|
I think that this is still a problem in the other SRM classes. Namely, DetSRM should handle intercept/bias vectors similarly as SRM. |
|
Other than that, the change for |
|
Thanks! @TuKo But does |
|
btw I think we should recommend people normalizing the data before using any SRM. Assuming the data is standardized, all SRMs can work well even without adjusting for subject-specific intercept terms. Of course, normalization also handles subject-specific scaling, which is also important. Fortunately, this seems to be what people have been doing. |
No, it doesn't. But it should have that option, if we are going to assume that data is not z-scored. |
|
I see. I just modified detsrm to allow it to handle intercepts. I also added a test to show that now detSRM can align subjects with different intercept terms (the same as the SRM intercept test). |
…h should save some memory...
…g 1 return var, but now it return both w and mu
|
There is another failed test that I don't understand. @TuKo do you know why it is failing? |
|
The unit tests are passing. The documentation building is failing, but I can fix that, don't worry about it. https://travis-ci.org/github/brainiak/brainiak/jobs/767874844 |
|
Sounds great! Thanks! @mihaic |
TuKo
left a comment
There was a problem hiding this comment.
All the code looks good. One small discrepancy between changes and code/documentation is left. Once it is corrected it should be ready to go.
| return U.dot(V), mu | ||
|
|
||
| def transform_subject(self, X): | ||
| """Transform a new subject using the existing model. |
There was a problem hiding this comment.
As the function _update_transform_subject() was modified to return mu, then we should update the documentation or code of transform_subject().
There was a problem hiding this comment.
thanks! what do you mean? I did update the doc string of transform_subject() about mu
This is a continuation of #406
I updated SRM to use the intercept term. Then in the srm test, I added an example showing that ...
X_i = W_i.T @ S + intercept_i
Then SRM can align them (only possible with the intercept term)
X_new = W_new.T @ S + intercept_new
Then srm.transform_subject() can align it to a trained shared response. It returns the estimated w and intercept (called
wandmu). A short comment about how to usewandmuis added to the doc string.I also added comments in detsrm, sssrm, rsrm about they current don't use the intercept term.
The auto-styling functionality of my editor added some spaces here and there, which shouldn't impact the functionality of the code.