MRG: Automatically selects EEG channels when plotting bridged electrodes on topomap

[mscheltienne]

I was playing a bit with the new bridged electrode detection feature introduced in #10571 and I ran into a couple of issues with the plot on topographic maps.

As for compute_bridged_electrodes, plot_bridged_electrodes should automatically selects the EEG channels. There was an attempt here:

mne-python/mne/viz/topomap.py

Line 2762 in 5944b87

picks = pick_types(info, eeg=True)

but it was not enough to prevent the function from raising a ValueError due to multiple channel types in the info when creating the topographic map.

Snippet that raises on main:

from mne.datasets import sample
from mne.io import read_raw_fif
from mne.io.pick import _picks_to_idx
from mne.preprocessing import compute_bridged_electrodes
from mne.viz import plot_bridged_electrodes


directory = sample.data_path() / "MEG" / "sample"
fname = directory / "sample_audvis_raw.fif"
raw = read_raw_fif(fname, preload=True)

# artificially bridge electrodes
picks = _picks_to_idx(raw.info, "eeg")
raw._data[picks[0]] = raw._data[picks[1]]

# determine and plot bridged electrodes
bridged_idx, ed_matrix = compute_bridged_electrodes(raw)  # copy and pick EEG
plot_bridged_electrodes(raw.info, bridged_idx, ed_matrix,
                        title='Bridged Electrodes', topomap_args=dict(vmax=5))

[mscheltienne]

I don't think the failures are related to this PR.

[agramfort]

LGTM

@alexrockhill merge if happy

thx @mscheltienne

[alexrockhill]

Looks good, one small nitpick.

[alexrockhill]

Nitpick but you call pick_info twice, would be nice just to do once.

[mscheltienne]

Yes, I thought the same, but I did not see a clean way to pick only once (at least not without changing more extensively the function). Problem is:

• you need the 'picked' info at the beginning to set the 'names' argument (alternatively [ch for k, ch in enumerate(info.ch_names) if k in picks].. not super clean and extends a lot this line).
• you need the 'picked' info when plotting the topomap with plot_topomap
• you need the 'non-picked' info at the end when adding the bridged connections because bridged_idx is based on the original info with all channels.

And the gain with the list comprehension for the 'name' argument is only ≈1 ms but loses in readability.
Feel free to add further changes if you have a clever way to pick only once while managing the bridged_idx, I didn't spend much time searching for one ;)

[mscheltienne]

Also @alexrockhill, another bug I want to address in a different PR (probably tomorrow as I'm getting started on this electrode bridging preprocessing step that was missing in my pipelines) ->

If you run the snippet below on main or if you run the code snippet in the first post that raises on main and is fixed in this PR (i.e. I did not break this 😅):

from mne.datasets import sample
from mne.io import read_raw_fif
from mne.preprocessing import compute_bridged_electrodes
from mne.viz import plot_bridged_electrodes


directory = sample.data_path() / "MEG" / "sample"
fname = directory / "sample_audvis_raw.fif"
raw = read_raw_fif(fname, preload=True)
raw.pick_types(eeg=True)

# artificially bridge electrodes
raw._data[0, :] = raw._data[1, :]

# determine and plot bridged electrodes
bridged_idx, ed_matrix = compute_bridged_electrodes(raw)  # copy and pick EEG
plot_bridged_electrodes(raw.info, bridged_idx, ed_matrix,
                        title='Bridged Electrodes', topomap_args=dict(vmax=5))

Outputs:

The line doesn't look like it's at the correct place ;)

EDIT: I actually dig a bit instead of shutting down the PC.. it has to do with the sphere radius that is off and not equal to the default 0.095 expected by this plotting function.

plot_bridged_electrodes(raw.info, bridged_idx, ed_matrix,
                        title='Bridged Electrodes', topomap_args=dict(vmax=5, sphere=0.095))

The code snippet above does force the sphere radius to the default value and does produce the correct figure. Without this, the sphere is set to array([-0.00413199, 0.0159844 , 0.05174612, 0.09100622]) instead of array([0. , 0. , 0. , 0.095]).

It looks like it's a combination of 2 bugs:

• The default sphere looks wrong.
• This function lacks support for different sphere arguments.

[alexrockhill]

@mscheltienne I'm not sure about the second bug, I'll have to look into it. Can we just use the sphere from topomap_args if passed? You didn't touch the example so it didn't execute. Can you either touch it and rerun CIs or confirm yourself locally that things look fine and then merge?

[hoechenberger]

The function name should go into a See Also section

[mscheltienne]

For the See Also section, it looked to me like usually it only contains function x-links without any text. Can you also put 'comment' like this one which mentions 2 of the arguments in the See Also section?

[mscheltienne]

@alexrockhill @hoechenberger The example runs correctly locally and the figures are correct. One of you can can merge if you don't have further comments :)
I'll have a look at the issue with sphere.

[agramfort]

+1 for MRG on my side when CIs are green

[mscheltienne]

I pushed a fix here for the sphere problem. The issue is:
-> plot_topomap calls _check_sphere(sphere)
-> _find_topomap_coords (to draw the line) calls _auto_topomap_coords which calls _check_sphere(sphere, info)

Notice the additional info that is now used to auto-fit a sphere in the second case..

from mne.datasets import sample
from mne.io import read_raw_fif
from mne.utils.check import _check_sphere


directory = sample.data_path() / "MEG" / "sample"
fname = directory / "sample_audvis_raw.fif"
raw = read_raw_fif(fname, preload=False)

_check_sphere(None, None)
# >>> array([0.   , 0.   , 0.   , 0.095])
_check_sphere(None, raw.info)
# >>> array([-0.00413199,  0.0159844 ,  0.05174612,  0.09100622])

[mscheltienne]

As green as it will get with the hash issue on the testing dataset.

[hoechenberger]

Pushed a small docstring change, otherwise LGTM!

[agramfort]

I think the CI failure is unrelated. @larsoner @drammock if you confirm maybe we can merge this one?

🙏 @mscheltienne

[drammock]

yes, this

Theme error:
An error happened in rendering the page auto_examples/connectivity/index.
Reason: PendingDeprecationWarning('nodes.Node.traverse() is obsoleted by Node.findall().')

is unrelated but disheartening, as I thought @larsoner and I eradicated that bug a week or two ago :( I'll look into it.

[alexrockhill]

Thanks @mscheltienne for fixing this, I really appreciate it!