MRG, ENH: Combine channels

[HanBnrd]

Reference issue

Closes #7984

What does this implement/fix?

Implementation of a way to combine channels from Raw, Epochs, or Evoked. Channels can be combined with a callable as argument or with "mean", "median", or "std".

Additional information

• It currently returns a new instance and does not modify the original one
• It is currently a method in mne/channels/channels.py, open to suggestions for a better location
• Was wondering if this sounds fine before implementing tests

[HanBnrd]

Hi @drammock,
I implemented this with a single dict as argument, would be happy to have some guidance to be also able to pass multiple key-values pairs. Should I do it with **kwargs? Would you know an example of this kind of function/method in MNE?

[drammock]

I had a quick look and the code looks reasonable (and is easy to read; thanks for using clear variable names!). See my comments below. For now let's not worry about the dict.update() approach, it would be nice to have but is not a blocker.

[HanBnrd]

I had a quick look and the code looks reasonable (and is easy to read; thanks for using clear variable names!). See my comments below. For now let's not worry about the dict.update() approach, it would be nice to have but is not a blocker.

Thanks for your detailed feedback :)

[agramfort]

2 things: - method should operate inplace - I am -1 on having both real EEG channels and virtual/combined EEG channels in the same object. 2 options: - use a function not a method that returns a new instance of an XxxArray class - remove all data / channels from the instance when adding these new signals. I am leaning towards a simple function.

…

[HanBnrd]

Just did the modifications based on your comments @drammock and @agramfort, thanks. Will implement tests now.

[HanBnrd]

I was actually thinking about turning verbose off when creating the new instances with XxxArray, is that something I should do with eg. RawArray(..., verbose=False)?

[larsoner]

I would set it to whatever the parent instance's .verbose value is

[HanBnrd]

I would set it to whatever the parent instance's .verbose value is

Thanks @larsoner, I could do that if that's fine. The thing is that it is creating a new instance with one channel for every ROI, the console may become flooded with many ROIs if verbose is on for the parent's instance.
The function appends an new instance with one channel at every loop iteration to gradually create the instance with all the ROIs. I have never worked with instances bigger than memory so far but I though (maybe wrongly) that it could eventually save memory to append instances instead of numpy arrays? If not, I can take a different approach where I append numpy arrays and finally create a new instance for better verbose?

[larsoner]

he thing is that it is creating a new instance with one channel for every ROI, the console may become flooded with many ROIs if verbose is on for the parent's instance.

I would create and return a single new instance with all the new (averaged) channels

[agramfort]

I would create and return a single new instance with all the new (averaged) channels

+1

[HanBnrd]

I changed the code to create a single new instance. I also implemented tests, could you please give me some guidance on that?

[rob-luke]

Looking great. I have one question about throwing an error if you try and combine less than two channels (one channel 😄).

Here is my situation. I have a dataset which I want to split in to two regions of interest. Each region of interest has 4 channels. But for one participant they have lots of long hair and a bad connection between sensors and the scalp. And for this participant 3 of the 4 channels in the ROI is marked as bad. However, the remaining channel is clean and usable. In this situation, to be consistent with the rest of the dataset, I would want to create an ROI even though it has one channel. I could write a custom script for this one participant, or have an if statement, but I think it would be cleaner to just use the same code and have a combined channel from one channel.

Could we perhaps print a warning to the user but not error in this situation?

[rob-luke]

I also implemented tests, could you please give me some guidance on that?

They look nice. I checked the codecov patch report and you have hit most of your lines of code. And the tests make sense and are documented well.

But because you asked... you have only tested with combining two channels (unless I have read wrong), so you could check it works for 3 + channels. You also check the mean result is the same a numpy. You could do the same for the other methods.

[HanBnrd]

Looking great. I have one question about throwing an error if you try and combine less than two channels (one channel 😄).

Here is my situation. I have a dataset which I want to split in to two regions of interest. Each region of interest has 4 channels. But for one participant they have lots of long hair and a bad connection between sensors and the scalp. And for this participant 3 of the 4 channels in the ROI is marked as bad. However, the remaining channel is clean and usable. In this situation, to be consistent with the rest of the dataset, I would want to create an ROI even though it has one channel. I could write a custom script for this one participant, or have an if statement, but I think it would be cleaner to just use the same code and have a combined channel from one channel.

Could we perhaps print a warning to the user but not error in this situation?

Yes you are right, thanks for pointing that out. I think that would be very handy, and I can see myself being in the same situation.

Actually, I'm thinking that also adding a parameter (eg. drop_bad as a boolean, default to False) to keep or drop bad channels when combining the ROIs would make this even easier. It would enable to specify only one ROI dict and iterate over many instances of Raw (or Epochs or Evoked) with different bad channels. It could eventually print a warning for dropped channels as well. What do you think?

EDIT: At the moment it is not taking care of bad channels and just considers them as normal channels which are combined if in the ROI dict.

[drammock]

last request. Can this be added to a tutorial or an example for documentation? maybe the EEG tutorial? basically to be put where you think users will expect to find it.

Sure, I think this function is probably suited for fNIRS, maybe in Preprocessing functional near-infrared spectroscopy (fNIRS) data?

Averaging channels isn't what I normally think of as "preprocessing". I think this tutorial is probably a good spot for it: https://mne.tools/dev/auto_tutorials/evoked/plot_eeg_erp.html It's one that I haven't had a chance to overhaul yet (I've been working through them all gradually); since I'll likely change the tutorial a lot when I get round to that one, any additions you make now could be pretty skeletal --- i.e., just a sentence saying "if you want to average across sensors to make an ROI, do this" and then a couple lines of code to average and then a plot. Since the tutorial already defines left and right evoked objects, maybe plot a left vs right ROI during a left stimulus?

[HanBnrd]

I added a few lines to the EEG processing and Event Related Potentials (ERPs) tutorial, please let me know if that's okay for you @drammock :)

Also some checks are failing, but I'm not sure if that's related to my changes...

[larsoner]

Windows 3.8 pip pre you can ignore for now, but CircleCI has a new warning (we don't allow warnings in tests EDIT the doc build):

https://app.circleci.com/pipelines/github/mne-tools/mne-python/2794/workflows/b19611f5-966c-4e98-8b27-04421717f505/jobs/21348

Traceback (most recent call last):
  File "/home/circleci/.local/lib/python3.8/site-packages/sphinx_gallery/gen_gallery.py", line 164, in call_memory
    mem, out = memory_usage(func, max_usage=True, retval=True,
  File "/home/circleci/.local/lib/python3.8/site-packages/memory_profiler.py", line 343, in memory_usage
    returned = f(*args, **kw)
  File "/home/circleci/.local/lib/python3.8/site-packages/sphinx_gallery/gen_rst.py", line 464, in __call__
    exec(self.code, self.fake_main.__dict__)
  File "/home/circleci/project/tutorials/evoked/plot_eeg_erp.py", line 117, in <module>
    raw_car, _ = mne.set_eeg_reference(raw, 'average', projection=True)
  File "<decorator-gen-211>", line 21, in set_eeg_reference
  File "/home/circleci/project/mne/io/reference.py", line 343, in set_eeg_reference
    warn('An average reference projection was already added. The data '
  File "/home/circleci/project/mne/utils/_logging.py", line 293, in warn
    warnings.warn_explicit(
RuntimeWarning: An average reference projection was already added. The data has been left untouched.

Which would be this line:

https://github.com/mne-tools/mne-python/pull/8014/files#diff-cdeb04d3326fc19e15cb55419752e417R117

You can either do verbose='error' or just write a comment saying that the data already have an average ref proj

[larsoner]

I killed Travis, it will fail until #8050 lands, then I'll restart CIs

[drammock]

Tutorial additions look reasonable. Pending the CIs getting straightened out (which I think @larsoner is working on) I'm +1 to merge this.

[larsoner]

@HanBnrd if you merge with master or rebase, CIs should fix themselves

[larsoner]

@HanBnrd let me know if you want help with the merge, but in theory you should just be able to do:

# git remote add upstream https://github.com/mne-tools/mne-python.git  <-- if you haven't already done this
git fetch upstream
git merge upstream master
git push

[HanBnrd]

@HanBnrd let me know if you want help with the merge, but in theory you should just be able to do:

# git remote add upstream https://github.com/mne-tools/mne-python.git  <-- if you haven't already done this
git fetch upstream
git merge upstream master
git push

Sorry @larsoner I'm not sure I can merge HanBnrd:combine-channels into mne-tools:master myself as I don't have write access to mne-tools/mne-python. I cannot do it from GitHub and it does not work from command line with your instructions. If I should normally be able to do it, I would be happy to have some help please.

[larsoner]

If I should normally be able to do it, I would be happy to have some help please.

To be clear, I'm not saying that you should push/merge your changes into github.com/mne-tools/mne-python.git / click the green button like maintainers do. What I'm saying is that on your local machine you can perform a merge, and push that merge commit here. You can merge any branch into any other branch on your local machine, there are no restrictions (on at least trying; sometimes there can be conflicts).

For example, I can do it locally on my machine:

larsoner@bunk:~/python/mne-python$ git remote add HanBnrd git@github.com:/HanBnrd/mne-python.git

larsoner@bunk:~/python/mne-python$ git fetch HanBnrd 
remote: Enumerating objects: 107, done.
remote: Counting objects: 100% (107/107), done.
remote: Total 133 (delta 107), reused 107 (delta 107), pack-reused 26
...

larsoner@bunk:~/python/mne-python$ git checkout -b combine-channels HanBnrd/combine-channels 
Branch 'combine-channels' set up to track remote branch 'combine-channels' from 'HanBnrd'.
Switched to a new branch 'combine-channels'
# Now I am where you are on your local machine

larsoner@bunk:~/python/mne-python$ git merge upstream/master
# It pops up a vim (for me) window, and I just `:wq` to save-and-quit -- the merge commit message is fine
Auto-merging mne/channels/channels.py
Auto-merging doc/python_reference.rst
Auto-merging doc/changes/latest.inc
Merge made by the 'recursive' strategy.
...

And at this point, I could now do:

git push

And it would push the merge commit to your branch (assuming I have permission to do so, which is the case if you checked the "allow contributions from maintainers").

But you should be able to do it, too. If you do:

git remote add upstream git://github.com/mne-tools/mne-python.git
git fetch upstream

Now your system knows about all up-to-date branches out on GitHub. Then if you're locally on your combine-channels branch and you do:

git merge upstream/master

Then save-and-exit the prompt that comes up about what merge commit message to use, then you should be able to do:

git push origin combine-channels

Then we should see a merge commit pop up in your branch in this PR.

[HanBnrd]

If I should normally be able to do it, I would be happy to have some help please.

To be clear, I'm not saying that you should push/merge your changes into github.com/mne-tools/mne-python.git / click the green button like maintainers do. What I'm saying is that on your local machine you can perform a merge, and push that merge commit here. You can merge any branch into any other branch on your local machine, there are no restrictions (on at least trying; sometimes there can be conflicts).

For example, I can do it locally on my machine:

larsoner@bunk:~/python/mne-python$ git remote add HanBnrd git@github.com:/HanBnrd/mne-python.git

larsoner@bunk:~/python/mne-python$ git fetch HanBnrd 
remote: Enumerating objects: 107, done.
remote: Counting objects: 100% (107/107), done.
remote: Total 133 (delta 107), reused 107 (delta 107), pack-reused 26
...

larsoner@bunk:~/python/mne-python$ git checkout -b combine-channels HanBnrd/combine-channels 
Branch 'combine-channels' set up to track remote branch 'combine-channels' from 'HanBnrd'.
Switched to a new branch 'combine-channels'
# Now I am where you are on your local machine

larsoner@bunk:~/python/mne-python$ git merge upstream/master
# It pops up a vim (for me) window, and I just `:wq` to save-and-quit -- the merge commit message is fine
Auto-merging mne/channels/channels.py
Auto-merging doc/python_reference.rst
Auto-merging doc/changes/latest.inc
Merge made by the 'recursive' strategy.
...

And at this point, I could now do:

git push

And it would push the merge commit to your branch (assuming I have permission to do so, which is the case if you checked the "allow contributions from maintainers").

But you should be able to do it, too. If you do:

git remote add upstream git://github.com/mne-tools/mne-python.git
git fetch upstream

Now your system knows about all up-to-date branches out on GitHub. Then if you're locally on your combine-channels branch and you do:

git merge upstream/master

Then save-and-exit the prompt that comes up about what merge commit message to use, then you should be able to do:

git push origin combine-channels

Then we should see a merge commit pop up in your branch in this PR.

Okay, thanks for the explanation :)

[drammock]

Here is the rendered tutorial. The plot looks odd; the two ROIs appear to be mirror images of one another?

[drammock]

Perhaps I'm being too nitpicky, but maybe a more realistic example is to hand-construct a dict of just a few channels from each side (usually an ROI is not "all electrodes on the left half of the head") instead of using the for-loop to sort by odd/even channel loc. WDYT?

[HanBnrd]

Yeah okay, I can select channels around the motor cortex left vs. right.

EDIT: my bad, I forgot it was auditory, so maybe around T3 and T4.

[HanBnrd]

This is what I have when I average those sensors (left vs. right) for left auditory:


What do you think?

[drammock]

That looks better. Thanks, and sorry for the last-minute change request... I didn't foresee (but should have) how strange the left half / right half ROI signals would look.

[agramfort]

LGTM https://21421-1301584-gh.circle-artifacts.com/0/dev/auto_tutorials/evoked/plot_eeg_erp.html#sphx-glr-auto-tutorials-evoked-plot-eeg-erp-py

[drammock]

thanks @HanBnrd!

[HanBnrd]

Thank you all for your support :)