[ENH] Add support for ieeg interpolation by alexrockhill · Pull Request #12336 · mne-tools/mne-python

alexrockhill · 2024-01-04T20:44:44Z

It also looked like there was a bug where epochs would have the channel indices of their epochs set to nan if nan was the method which was also fixed.

for more information, see https://pre-commit.ci

…into interp2

alexrockhill · 2024-01-04T21:31:09Z

The seeg interpolation looks pretty good.

alexrockhill · 2024-01-04T21:31:58Z

mne/channels/interpolation.py

        picks_bad = pick_channels(inst.info["ch_names"], bads_type, exclude=[])

-        if method[ch_type] == "nan":
-            inst._data[picks_bad] = np.nan


This is the line with the bug as far as I can tell, this should only work on raw and have some odd and unexpected indexing behavior on epochs

As opposed to here https://github.com/mne-tools/mne-python/pull/12336/files#diff-fcfbd5c9390aa74bdd3005e5ae1c786df72afc324d3532273dc5937ef9674787L130-R131 which does it correctly

alexrockhill · 2024-01-04T21:43:47Z

Confirmed, this is incorrect on main.

import mne
raw = mne.io.read_raw(mne.datasets.sample.data_path() / 'MEG' / 'sample' / 'sample_audvis_raw.fif')
events = mne.find_events(raw)
epochs = mne.Epochs(raw, events)
epochs.info['bads'] = epochs.ch_names[:10]
epochs.load_data()
epochs.interpolate_bads(method=dict(meg='nan'), reset_bads=False)

epochs._data[20, 0]  # should be nan
epochs._data[0, 20]  # shouldn't be nan but is

alexrockhill · 2024-01-04T21:48:47Z

Oh odd, if you try the above code snippet on this branch, the data is correct but when you call epochs.plot it'll all blank and you get

mne-python/mne/viz/_figure.py:372: RuntimeWarning: Mean of empty slice
  data -= np.nanmean(data, axis=1, keepdims=True)

Looks like an unrelated bug.

alexrockhill · 2024-01-05T16:05:57Z

Looks like everything works as expected. This fixes both the ieeg quiet non-interpolations and the newly discovered nan bug. Once this merges, I will open an issue for the epochs browser bug but it would just confuse things to open it depending on main now because of the nan bug.

drammock

nice PR, thanks! A few questions/suggestions; I ran out of time and didn't thoroughly check the tests FWIW, so should get at least one other review (or another pass from me next week)

mne/channels/channels.py

drammock · 2024-01-05T22:26:27Z

mne/channels/interpolation.py

+    inst.info._check_consistency()
+    bads_idx[picks] = [inst.ch_names[ch] in inst.info["bads"] for ch in picks]
+
+    if len(picks) < 3 or bads_idx.sum() == 0:


[question] IDK how much of an edge case this is or if it's a realistic worry, but should we warn if len(picks) < 3? I can imagine maybe being surprised that the single bad channel wasn't interpolated because I (inadvertantly?) excluded all but one of the good channels (leaving just 2 channels in picks)

Yeah, I think maybe best just to kick it to the runtime error if there's a bad channel on a shaft with less than 3 contacts which is the edge case and I think an error is warranted there since that's not a good interpolation situation. So no need to silently return.

I think a warning is better. If you have 3 bad channels and this edge-case represents only one of them, the interpolation will fail and raise unless you mark the edge-case bad channel as good, interpolate, and mark it again as bad. I find this more error prone than warning, interpolating the 2 other channels and ignoring the edge-case one, and finally removing only the 2 interpolated channels from .info["bads"].

This won't raise an error if there are 3 bad channels on a shaft (electrode) only if there aren't three good ones. If there are only two good channels, I don't think there is a use case where you want to use those to interpolate any of the bad channels so I'd still be disposed to an error. Although if you had 3 contacts (usually 4 for DBS) but as in a DBS and you wanted to interpolate the middle one on some epochs that would make sense so maybe it should just be two required.

I mixed up bad channels/good ones in my comment. I agree there is no use case for interpolating along a shaft if you lack the information to do so. But I would still warn, else if you have 2 shafts each with some bad channels, one with less than 3 good contacts remaining; the interpolation function will fail even for the shaft which has enough information to interpolate.

raw.ch_names = [f"Shaft1_{k} for k in range(5)] + [f"Shaft2_{k} for k in range(5)] raw.info["bads"] = [f"Shaft1_{k} for k in range(4)] + ["Shaft2_0"] raw.interpolate_bads(...) # warning for the bad contacts on Shaft1, interpolation for Shaft2_0 assert raw.info["bads"] == [f"Shaft1_{k} for k in range(4)]

Or am I misunderstanding something and/or a multi-shaft recording is not realistic? (I lack experience with intra-cranial recordings)

The length of the shaft isn't checked if the channels aren't marked bad so I think if you do mark it and try and interpolate it you should get an error rather than unreliable data. You would then have to drop that channel without interpolation to continue without an error which I think is the safest way to go and wouldn't cause major issues in multi-subject pipelines.

I think the other consideration if it's a warning is what do you do to the data in that channel? If it was supposed to be interpolated and you just warned and the data was left as is, that seems potentially misleading. If it only requires two other contacts, I think it's pretty safe that this is an unlikely edge case and the failure will be warranted so that the user can be notified to fix their data by dropping all the contacts on that electrode shaft.

mne/channels/interpolation.py

alexrockhill · 2024-01-06T00:42:40Z

Thanks for the review, some nice cleanups

doc/changes/devel/12336.bugfix.rst

mne/channels/interpolation.py

alexrockhill · 2024-01-08T18:59:21Z

mne/channels/interpolation.py

+    inst.info._check_consistency()
+    bads_idx[picks] = [inst.ch_names[ch] in inst.info["bads"] for ch in picks]
+
+    if len(picks) < 3 or bads_idx.sum() == 0:


The length of the shaft isn't checked if the channels aren't marked bad so I think if you do mark it and try and interpolate it you should get an error rather than unreliable data. You would then have to drop that channel without interpolation to continue without an error which I think is the safest way to go and wouldn't cause major issues in multi-subject pipelines.

alexrockhill · 2024-01-08T23:57:35Z

Good to go by me, no rush though

mne/channels/tests/test_interpolation.py

agramfort · 2024-01-09T08:14:51Z

mne/channels/interpolation.py

+    np.fill_diagonal(dist, np.inf)
+
+    picks_bad = list(np.where(bads_idx_pos)[0])
+    while picks_bad:


can we avoid while loops and use a plain for? I find them usually unecessary and error prone.

The while loop was used because you don't know if two bad channels are on the same shaft until you find the shaft. I can't think of an easy way around that with a for loop.

I wonder if a helper function (_find_shafts or so) that returns a list-of-lists of channel indices would be generally useful? If so, it could be used here to pre-compute the channels into shaft groupings, then a for-loop over picks_bad would work.

Sure, we could do that. I was just finding them for the bad contacts to save the computation but it's a very small amount

Hmm, I wrote that but even if you know the shafts, you still have to iterate over picks_bad (list of bad indices) and you don't know whether two are on the same shaft or not so you have to use a while loop.

Oh never mind you can just loop over shafts

alexrockhill · 2024-01-09T19:48:55Z

Here are comparisons from a middle contact (left) and and edge contact (right) interpolated (blue) vs original (orange). Looks pretty reasonable.

This is 10 s of real data.

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

[ENH] Add support for ieeg interpolation

3264e36

alexrockhill requested review from agramfort, dengemann, jasmainak and mscheltienne as code owners January 4, 2024 20:44

pre-commit-ci bot and others added 5 commits January 4, 2024 20:47

[pre-commit.ci] auto fixes from pre-commit.com hooks

0376bd7

for more information, see https://pre-commit.ci

update changes, fix avoid extra computations

2414412

style

3571a80

style

d6a284a

Merge branch 'interp2' of https://github.com/alexrockhill/mne-python …

1dfe265

…into interp2

alexrockhill requested review from drammock and larsoner as code owners January 4, 2024 21:27

alexrockhill commented Jan 4, 2024

View reviewed changes

alexrockhill added 3 commits January 4, 2024 13:49

fix test

747c448

fix tests

7fb871c

fix tests

0084a36

drammock reviewed Jan 5, 2024

View reviewed changes

alexrockhill added 2 commits January 5, 2024 16:31

Dan review

2a88eb0

small fix

70c9770

agramfort reviewed Jan 7, 2024

View reviewed changes

doc/changes/devel/12336.bugfix.rst Outdated Show resolved Hide resolved

alexrockhill commented Jan 8, 2024

View reviewed changes

alexrockhill added 2 commits January 8, 2024 11:06

review comments

0a8af16

rerun ci

4df0888

agramfort reviewed Jan 9, 2024

View reviewed changes

alexrockhill added 4 commits January 9, 2024 10:04

Alex review

6d2d537

fix docstring

c3f2679

Merge branch 'main' into interp2

5ffcf2b

refactor

2d872f2

drammock approved these changes Jan 9, 2024

View reviewed changes

agramfort approved these changes Jan 10, 2024

View reviewed changes

agramfort merged commit f70378a into mne-tools:main Jan 10, 2024

alexrockhill deleted the interp2 branch January 10, 2024 17:57

snwnde pushed a commit to snwnde/mne-python that referenced this pull request Mar 20, 2024

[ENH] Add support for ieeg interpolation (mne-tools#12336)

1b768e9

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

Uh oh!

Conversation

alexrockhill commented Jan 4, 2024

Uh oh!

alexrockhill commented Jan 4, 2024

Uh oh!

alexrockhill Jan 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexrockhill commented Jan 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexrockhill commented Jan 4, 2024

Uh oh!

alexrockhill commented Jan 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

drammock left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mscheltienne Jan 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alexrockhill commented Jan 6, 2024

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexrockhill commented Jan 8, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexrockhill commented Jan 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

alexrockhill Jan 4, 2024 •

edited

Loading

alexrockhill commented Jan 4, 2024 •

edited

Loading

alexrockhill commented Jan 5, 2024 •

edited

Loading

mscheltienne Jan 6, 2024 •

edited

Loading

alexrockhill commented Jan 9, 2024 •

edited

Loading