-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Add more referencing options #1670
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
mne/channels/channels.py
Outdated
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.
@wmvanvliet which problem does this solve?
mne/io/base.py
Outdated
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.
eeg -> EEG
|
should we have a way to store in info or somewhere what type of referencing was applied? How about a method to revert a reference. I am thinking about this for inverse modeling as you don't want anything else than average ref for inverse modeling. This undo_reference or revert_reference method should be able to be applied to any data container. |
|
It would be very hard to invert referencing and apply CAR retroactively. For example, channels may be dropped along the way. If the user wants to perform source modelling, they should definitely stick to a CAR reference, which is and should be the default. The user has to explicitly define another reference and presumably the user knows what he/she is doing. |
you're optimistic :) Then we should have a flag that says if data has been referenced is a |
+1 |
|
I can add the flag, but the check should be implemented by someone familiar with the source modelling code. |
|
sounds like a plan |
|
fair enough |
|
Shall I add a constant to io.constants.FIFF for some referencing types (including OTHER)? Then source modelling code can check |
|
how many of such options would we add? other option is have a boolean flag. |
|
but we must now what kind of reference was applied, CAR or something else…
Marijn van Vliet |
the thing is that CAR works with an SSP / proj and it's nice and elegant. I feel we should stick to it when we apply CAR (ie. create proj and apply it) For all the other reference options set a flag that says that it will wdyt? |
|
Agreed. Lets recap the EEG referencing arguments so far:
1. The default reference should be CARUpon loading a raw file, a CAR projector is automatically added for the EEG. Any bad channels already marked in the raw data are excluded. 2. It should be straightforward for the user to apply another referenceThe If a CAR SSP projection is present in the data, it should be removed by this function. Otherwise things become very confusing for the user. When an instance of 3. When another reference is set, source localization will be unreliable/wrongThe user might not be aware of this, so a flag should be set if a non-CAR reference is applied. An error can then be generated during source localization. Lets keep the code simple and make 4. Computing the CAR on bad channels is badThere should be a way to conveniently update the CAR projection after modifying the @dengemann @agramfort @t3on Do you agree on all points? |
|
regarding #1 the only thing is that you often do not know about bad channels before loading data, often you only tknow that at the epochs stage. apart from that sounds good
|
|
yess, 4. is exactly it
|
should we deprecate this default behavior?
I feel when CAR is asked the proj should be added and applied. It is
+1
I would set it to True only if non CAR.
I am -1 on this.
hide the complexity from the user. does it seem doable?
+1. Any suggestion of how to do this? |
Probably not. It would open up the possibility for the user to forget to apply a reference. If we can make it easy for the user to set another reference, this default should not inconvenience the user.
But how should the user ask for CAR? Of course, the best method for the user would be to simply keep the default. But if the user really wants to explicitly ask for it, I was thinking of making him call the
yes.
I was thinking about putting a system in place that checks the info dictionary. When it is changed (for example, channel names are updates, channels are added or removed) the dictionary is updated (nchans, ch_names, etc.) Such a system could also keep track of changes to the bad channels. Definitely not something for this PR, but something to think about. |
|
what does the |
|
@agramfort how about this for an API to |
when a proj is active it means it has been applied to the data. |
|
besides LGTM you'll need to update python_reference.rst + what's new someone please have a look too. |
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.
So this field always exists when reading info now...
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.
yes, otherwise you'll have to check if 'custom_ref_applied' in info and info['custom_ref_applied'] every time you want to do something with the flag.
|
Other than my minor concerns, LGTM. |
mne/io/base.py
Outdated
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.
pep257
One headline after """
then empty line and a full sentence / paragraph describing the function.
|
also:
that's it for me now. |
fea301a to
268b44e
Compare
|
Rebased. |
examples/plot_rereference_eeg.py
Outdated
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.
Use the mean of channels EEG 001 and EEG 002 as a reference
This adds a base function 'apply_reference' that can apply various different referencing schemes. Two convenience functions are also added that provide a convenient interface to the base function: - set_eeg_reference: apply a simple reference to all EEG channels - set_bipolar_reference: apply a bipolar reference (or more)
Raw and Epoch classes will no longer add an average reference if the `custom_ref_applied` flag was set. SSP projections will still attempt this, but a new assertion in `make_eeg_average_ref_proj` will raise an error if the flag was set. Before a CAR projection is added, the `info['custom_ref_applied']` flag is checked.
- Also check for duplicate channel names - Bilpolar channels can be named after the anode/cathode - Inform user of the chosen bipolar channel name
7b6f1e6 to
21c2227
Compare
|
rebased (again), and tweaked the example a little to reduce memory consumption. |
|
closing in favor of #1713 @wmvanvliet see my last 4 commits. |
This adds a base function
apply_referencethat can apply variousdifferent referencing schemes. Two convenience functions are also
added that provide a convenient interface to the base function:
set_eeg_reference: apply a simple reference to all EEG channelsset_bipolar_reference: apply a bipolar reference (or more)A helper function
_parse_channel_listis also added to help functionsbe more flexible in how they allow channels to be specified.