Bug in interpolate_blinks

[aabielefeldt]

Description of the problem

Bug in interpolate_blinks:

The interpolate_blinks function asks for inputs to "match":
"description of annotations to interpolate over. If a list, the data within all annotations that match any of the strings in the list will be interpolated over."

However the helper function _interpolate_blinks hardcodes the description to "BAD_blink" and doesn't use the inputs entered in "match". Therefore any interpolated blinks, no matter which description is entered in match use the start and end times of any "BAD_blink" annotations.

line 91:
starts, ends = _annotations_starts_stops(raw, "BAD_blink")

Solution:

I rewrote the function to avoid this issue by removing the line defining the starts and ends with the _annotations_starts_stops function and instead just used the “blink_annots” created in the function to define starts and stops of the annotations to interpolate over:

        # Create an empty boolean mask
        mask = np.zeros_like(raw.times, dtype=bool)

        # for annot, start, end in zip(blink_annots, starts, ends):
        for annot in blink_annots:
            if "ch_names" not in annot or not annot["ch_names"]:
                msg = f"Blink annotation missing values for 'ch_names' key: {annot}"
                raise ValueError(msg)
            if ch_info["ch_name"] not in annot["ch_names"]:
                continue  # skip if the channel is not in the blink annotation
            # Update the mask for times within the current blink period
            start = annot["onset"] - pre_buffer
            end = annot["onset"] + annot["duration"] + post_buffer
            
            mask |= (raw.times >= start) & (raw.times <= end)

I also added other interpolation methods to choose from and made the function ignore NaNs in the data. I’ve attached the function below in case this should be useful.

interpolate_blinks_function_custom.rtf

Steps to reproduce

raw = interpolate_blinks(raw, buffer=[0.05, 0.05], match=["BAD_velo_blink", "BAD_merged_blink"], interpolate_gaze=False)

-> would only interpolate over time periods where BAD_blink is annotated. This is especially confusing when blinks are automatically annotated from the eyetracker used (e.g. EyeLink)

Link to data

No response

Expected results

Actual results

Additional information

MNE version: 1.9.0

Operating system (on remote server connected to via ssh): Ubuntu 20.04.6

[scott-huberty]

However the helper function _interpolate_blinks hardcodes the description to "BAD_blink" and doesn't use the inputs entered in "match".

Oops! Thanks for the catch!

ine 91: starts, ends = _annotations_starts_stops(raw, "BAD_blink")

Solution:

I rewrote the function to avoid this issue by removing the line defining the starts and ends with the _annotations_starts_stops

If I recall correctly we do need starts, ends = _annotations_starts_stops(raw, "BAD_blink") to handle edge cases where the raw object has a non-zero orig time, for example the sample dataset:

import mne
fpath = mne.datasets.sample.data_path() / "MEG" / "sample" / "sample_audvis_raw.fif"
raw = mne.io.read_raw_fif(fpath)
print(raw.first_time)

42.95597082905339

Can we do something like starts, ends = _annotations_starts_stops(raw, this_match) ?

[scott-huberty]

xref #12408 (issue) and #12759 (Fix)

[aabielefeldt]

Hey,
sorry for only getting back to this now. You are absolutely right. On the test data, as well as on my data if I crop it before interpolation, it does not work with my solution anymore.

Originally I did use _annotations_start_stops(raw, match) and that did seem to work fine.
I was worried however about the way that the "blink_annots" created above are linked to the starts and stops from _annotations_start_stops in the snippet below:

for annot, start, end in zip(blink_annots, starts, ends):
            if "ch_names" not in annot or not annot["ch_names"]:
                msg = f"Blink annotation missing values for 'ch_names' key: {annot}"
                raise ValueError(msg)

While this seems to work, it does necessitate that they are in the same order, right?
-> meaning blink_annots[i] needs to refer to the same annotation as starts[i], ends[i].

Also the annotations in _annotations_start_stops are matched by prefix, while the blink_annots are defined based on exact description matches (annot["description"] in match). So if my list would have something like "BAD_blink", "BAD_blink_right", "BAD_blink_left", these would all be providing the starts and ends to be interpolated whether I specified them in match or not and would probably break the matched order then too since the number of annotations between blink_annots and annotations contributing to starts and stops would differ. So there could easily be a mismatch between the blink_annots and the starts and stops.

Since match is a list in my case I was worried it might result in such a mismatch. Maybe this is unlikely but definitely something to keep in mind.

[scott-huberty]

@aabielefeldt Thanks for the explanation and I think your diagnosis is probably true. Do you have time to help fix your issue? I think that if we start with a PR that simply addresses the "BAD_blink" hard coding (and adds a test to demonstrate that match is respected, given that match is a str), then we can iterate from there to also make sure that match is respected when a user passes a list of str.