Repository navigation
Noisy resolution matrix - #11092
Noisy resolution matrix#11092olafhauk wants to merge 39 commits into
Conversation
Conflicts: doc/changes/latest.inc mne/minimum_norm/resolution_matrix.py mne/minimum_norm/tests/test_resolution_matrix.py
|
This is just to get started where I left off last year. |
|
@olafhauk would it help if I pushed a commit to fix the conflicts so that locally you can do |
wmvanvliet
left a comment
wmvanvliet
left a comment
There was a problem hiding this comment.
Looks super good already @olafhauk! Just some questions.
| Note: If the resolution matrix is computed with a noise covariance | ||
| matrix then only its columns, i.e. PSFs, can meaningfully be | ||
| interpreted. It must not be used to compute CTFs or resolution metrics | ||
| for CTFs! |
There was a problem hiding this comment.
Perhaps this explanation of the role of the noise parameter is best moved to the "main" section of the documentation, above the Parameters subheading. Right now, the main section is only a single line, and then the lines describing the parameters and return values are filed with a large amount of text.
There was a problem hiding this comment.
I'll think about this later. I agree currently it may be confusing.
| idx_ch_types = {} | ||
| idx_ch_types["eeg"] = pick_types(info, eeg=True, meg=False) | ||
| idx_ch_types["mag"] = pick_types(info, eeg=False, meg="mag") | ||
| idx_ch_types["gra"] = pick_types(info, eeg=False, meg="grad") |
There was a problem hiding this comment.
Would there ever be a scenario that there would be other sensor types as well? Perhaps not.
There was a problem hiding this comment.
I don't know - are there other sensor types? How are OPMs dealt with?
Is there a method to get all sensor types from info and loop through them?
|
|
I just managed to close the pull request. Is there a way to reopen it, or should I create a new one? |
|
In principle I think you also had a "Reopen pull request" button but in case you didn't, I did so I used it :) |
for more information, see https://pre-commit.ci
Thank you. Good to be back ;-) |
…hon into noisy_resolution_matrix
…hon into noisy_resolution_matrix
|
@olafhauk All tests pass now. Are we happy with how the example renders? |
…hon into noisy_resolution_matrix
Thanks for contributing a pull request! Please make sure you have read the
contribution guidelines
before submitting.
Please be aware that we are a loose team of volunteers so patience is
necessary. Assistance handling other issues is very welcome. We value
all user contributions, no matter how minor they are. If we are slow to
review, either the pull request needs some benchmarking, tinkering,
convincing, etc. or more likely the reviewers are simply busy. In either
case, we ask for your understanding during the review process.
Again, thanks for contributing!
Reference issue
Implements #11058
What does this implement/fix?
Added "noisy resolution matrix" for inverse operators.
Additional information
Work in progress, just for initial review.