Conversation
includes tests and examples
And move pairwise_correlation
new comparison example
bug fixes, doc string
but fix
Improved plots, stats
gavinmischler
left a comment
gavinmischler
left a comment
There was a problem hiding this comment.
Pretty useful functionality! Just a few comments, and we should try to get the doc-render working
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #222 +/- ##
==========================================
+ Coverage 74.03% 74.41% +0.37%
==========================================
Files 58 60 +2
Lines 4287 4718 +431
==========================================
+ Hits 3174 3511 +337
- Misses 1113 1207 +94 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-authored-by: Gavin Mischler <gavinmischler@gmail.com>
Bug fix on coef()
gavinmischler
left a comment
gavinmischler
left a comment
There was a problem hiding this comment.
Sorry, last things I didn't realize before, otherwise I think it's ready!
| processed_trials.append(delayed.reshape(delayed.shape[0], -1)) | ||
| return processed_trials | ||
|
|
||
| def fit(self, data, feature_order, target='resp'): |
There was a problem hiding this comment.
Sorry, forgot to comment this earlier, but in general it's better if arguments to functions like this which take data also specify all the other args. See TRF.fit() or TRF.predict() for an example. This is helpful because it explicitly shows all the columns of data that are required by the model, otherwise it's unclear what the data should look like. And all args should be kwargs, so data=None is the default. Then, the user can call the function by passing data, or by passing each arg separately, or a combination. This is the purpose of the _parse_outstruct_args function.
This should be done for this and for predict()
There was a problem hiding this comment.
Gotcha, makes sense. Will change
| self.tmax = tmax | ||
| self.sfreq = sfreq | ||
| self.alphas = alphas if alphas is not None else np.logspace(-2, 5, 8) | ||
| self.basis_dict = basis_dict if basis_dict is not None else {} |
There was a problem hiding this comment.
One more feature to add--implement function to automatically produce BSpline basis dict for specified non-linearity (ala Unfold) and validate that basis functions are properly applied to features
| X_mats = self._prepare_matrix(feat_data_list, requested_features, self.feature_alphas_) | ||
| n_trials = len(X_mats) | ||
|
|
||
| if n_trials != len(self.model_): |
There was a problem hiding this comment.
Will also change BandedTRF object to use LOTO CV as default, but also allow for k-fold CV using concatenated and re-segmented data
2 participants
Reference Issues/PRs
What does this implement/fix? Explain your changes.
Adds BandedTRF class for modeling features using TRFs trained with banded ridge regression.
Includes examples showing 1. That banded regularization outperforms standard TRFs by determining regularization separately for each feature and 2. BandedTRFs are dependent on the order of features used
Any other comments?