-
Notifications
You must be signed in to change notification settings - Fork 140
Mostly bug fixing for brsa #144
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
…eals with shared time series not explained by design matrix
Fixed some bugs in utils.ReadDesign Renamed epsilon to eta in consistence with the paper Expanded the example a bit Other minor changes
…eals with shared time series not explained by design matrix
Changes to be committed: modified: brainiak/reprsimil/brsa.py modified: brainiak/utils/utils.py modified: examples/reprsimil/brsa_representational_similarity_estimate_example.ipynb modified: tests/reprsimil/test_brsa.py modified: tests/utils/test_utils.py
…nto spatial_corr
logger.info('You did not provide time seres of no interest ' | ||
|
||
logger.info('You did not provide time series of no interest ' | ||
|
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.
You probably do not want these empty lines.
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! fixed
LL, LAMBDA_i, LAMBDA, YTAcorrXL_LAMBDA, sigma2 \ | ||
= self._calc_LL(rho1, LTXTAcorrXL, LTXTAcorrY, YTAcorrY, | ||
X0TAX0, SNR2, n_V, n_T, n_run, rank, n_base) | ||
except: |
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.
You must specify a particular exception and you must add that exception to the docstring of the function in a section called "Raises".
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.
-->OK I removed the try/except
logger.debug('YTAcorrY: {}'.format(YTAcorrY)) | ||
logger.debug('LTXTAcorrY: {}'.format(LTXTAcorrY)) | ||
logger.debug('YTAcorrXL_LAMBDA: {}'.format(YTAcorrXL_LAMBDA)) | ||
logger.debug('SNR2: {}'.format(SNR2)) |
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.
Have you thought if warnings.warn()
or logger.warning
are appropriate here?
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.
Here are some guidelines for when to use what:
https://docs.python.org/3/howto/logging.html#when-to-use-logging
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 changed to logger.warning
@@ -7,7 +7,7 @@ | |||
}, | |||
"source": [ | |||
"# This demo shows how to use the Bayesian Representational Similarity Analysis method in brainiak with a simulated dataset.\n", | |||
"Questions can be directed to mcai [ at ] princeton [ dot ] edu" | |||
"*Feedbacks and questions are welcome. Please direct them to mcai [ at ] princeton [ dot ] edu" |
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.
Not really what the change is about, but do you mind removing this line? We are already instructing users to open GitHub issues for BrainIAK or contact us on Gitter.
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.
OK!
And some typo corrections.
The fitV and fitU steps in the _fit_diagV_noGP and _fit_diagV_GP functions are swapped when debugging. But I think the order does not matter so I did not swap them back.
Update example notebook to mention that z-scoring data is necessary.