mne-tools / mne-python

MNE: Magnetoencephalography (MEG) and Electroencephalography (EEG) in Python
https://mne.tools
BSD 3-Clause "New" or "Revised" License
2.7k stars 1.31k forks source link

set_channel_types does not properly set the coil_type for eyetracking channels #12746

Open scott-huberty opened 2 months ago

scott-huberty commented 2 months ago

Issue 1 (Originally posted in https://github.com/mne-tools/mne-python/pull/12744#issue-2425236972 ):

I am important eye tracking data from a tobii device (XDF format), and kept running into this issue when trying to set_channel_types:

KeyError: 0 (FIFFV_COIL_NONE) After some digging, I found that there is a different function that I should call. With this PR I want to make this more obvious in the documentation. However, the above issue also points out that there is probably a bug when setting channel types for eye-tracking.


MWE

The code below fails because our use of set_channel_types does not set the coil_type for eyetracking channels:

import numpy as np
import mne

shape = (1, 100)
data = np.vstack([np.full(shape, 960), np.full(shape, 540), np.full(shape, 0)])

info = mne.create_info(
        ch_names=["xpos", "ypos", "pupil"],
        sfreq=100,
        ch_types="eeg"
    )
raw = mne.io.RawArray(data, info)
raw.set_channel_types(dict(xpos="eyegaze", ypos="eyegaze", pupil="pupil"))
epochs = mne.make_fixed_length_epochs(raw)
Stack Trace ``` --------------------------------------------------------------------------- KeyError Traceback (most recent call last) Cell In[1], line 14 12 raw = mne.io.RawArray(data, info) 13 raw.set_channel_types(dict(xpos="eyegaze", ypos="eyegaze", pupil="pupil")) ---> 14 epochs = mne.make_fixed_length_epochs(raw) File :12, in make_fixed_length_epochs(raw, duration, preload, reject_by_annotation, proj, overlap, id, verbose) File ~/devel/repos/mne-python/mne/epochs.py:4967, in make_fixed_length_epochs(raw, duration, preload, reject_by_annotation, proj, overlap, id, verbose) 4965 events = make_fixed_length_events(raw, id=id, duration=duration, overlap=overlap) 4966 delta = 1.0 / raw.info["sfreq"] -> 4967 return Epochs( 4968 raw, 4969 events, 4970 event_id=[id], 4971 tmin=0, 4972 tmax=duration - delta, 4973 baseline=None, 4974 preload=preload, 4975 reject_by_annotation=reject_by_annotation, 4976 proj=proj, 4977 verbose=verbose, 4978 ) File :12, in __init__(self, raw, events, event_id, tmin, tmax, baseline, picks, preload, reject, flat, proj, decim, reject_tmin, reject_tmax, detrend, on_missing, reject_by_annotation, metadata, event_repeated, verbose) File ~/devel/repos/mne-python/mne/epochs.py:3568, in Epochs.__init__(self, raw, events, event_id, tmin, tmax, baseline, picks, preload, reject, flat, proj, decim, reject_tmin, reject_tmax, detrend, on_missing, reject_by_annotation, metadata, event_repeated, verbose) 3563 events, event_id, annotations = _events_from_annotations( 3564 raw, events, event_id, annotations, on_missing 3565 ) 3567 # call BaseEpochs constructor -> 3568 super().__init__( 3569 info, 3570 None, 3571 events, 3572 event_id, 3573 tmin, 3574 tmax, 3575 metadata=metadata, 3576 baseline=baseline, 3577 raw=raw, 3578 picks=picks, 3579 reject=reject, 3580 flat=flat, 3581 decim=decim, 3582 reject_tmin=reject_tmin, 3583 reject_tmax=reject_tmax, 3584 detrend=detrend, 3585 proj=proj, 3586 on_missing=on_missing, 3587 preload_at_end=preload, 3588 event_repeated=event_repeated, 3589 verbose=verbose, 3590 raw_sfreq=raw_sfreq, 3591 annotations=annotations, 3592 ) File :12, in __init__(self, info, data, events, event_id, tmin, tmax, baseline, raw, picks, reject, flat, decim, reject_tmin, reject_tmax, detrend, proj, on_missing, preload_at_end, selection, drop_log, filename, metadata, event_repeated, raw_sfreq, annotations, verbose) File ~/devel/repos/mne-python/mne/epochs.py:668, in BaseEpochs.__init__(***failed resolving arguments***) 666 self.reject = None 667 self.flat = None --> 668 self._reject_setup(reject, flat) 670 # do the rest 671 valid_proj = [True, "delayed", False] File ~/devel/repos/mne-python/mne/epochs.py:797, in BaseEpochs._reject_setup(self, reject, flat, allow_callable) 795 def _reject_setup(self, reject, flat, *, allow_callable=False): 796 """Set self._reject_time and self._channel_type_idx.""" --> 797 idx = channel_indices_by_type(self.info) 798 reject = deepcopy(reject) if reject is not None else dict() 799 flat = deepcopy(flat) if flat is not None else dict() File ~/devel/repos/mne-python/mne/_fiff/pick.py:873, in channel_indices_by_type(info, picks) 871 picks = _picks_to_idx(info, picks, none="all", exclude=(), allow_empty=True) 872 for k in picks: --> 873 ch_type = channel_type(info, k) 874 for key in idx_by_type.keys(): 875 if ch_type == key: File ~/devel/repos/mne-python/mne/_fiff/pick.py:255, in channel_type(info, idx) 253 if first_kind in _second_rules: 254 key, second_rule = _second_rules[first_kind] --> 255 first_kind = second_rule[ch[key]] 256 return first_kind KeyError: 0 (FIFFV_COIL_NONE) ```

However the code below does work because mne.create_info properly sets the coil_type for all channels:

code ``` # Note that I do not use set_channel_types here import numpy as np import mne shape = (1, 100) data = np.vstack([np.full(shape, 960), np.full(shape, 540), np.full(shape, 0)]) info = mne.create_info( ch_names=["xpos", "ypos", "pupil"], sfreq=100, ch_types=["eyegaze", "eyegaze", "pupil"] ) raw = mne.io.RawArray(data, info) epochs = mne.make_fixed_length_epochs(raw) ```

Issue 2

One work around to the issue above is to use set_channel_types_eyetrack, like so:

Code ``` import numpy as np import mne shape = (1, 100) data = np.vstack([np.full(shape, 960), np.full(shape, 540), np.full(shape, 0)]) info = mne.create_info( ch_names=["xpos", "ypos", "pupil"], sfreq=100, ch_types=["eyegaze", "eyegaze", "pupil"] ) more_info = dict( xpos=("eyegaze", "px", "right", "x"), ypos=("eyegaze", "px", "right", "y"), pupil=("pupil", "au", "right"), ) raw = mne.io.RawArray(data, info) # This sets the coil types, loc array, etc. mne.preprocessing.eyetracking.set_channel_types_eyetrack(raw, more_info) epochs = mne.make_fixed_length_epochs(raw) ```

However I personally think that This function has drawbacks:


Proposal

Maybe we can start with fixing issue Number 1, i.e. make sure that set_channel_types properly sets the coil_type for eyetracking channels. And then loop back to see if we can improve the API for setting other info for eyetracking channels, like the loc array, etc.

sappelhoff commented 2 months ago

Thanks Scott, this perfectly summarizes the issues I was facing.

larsoner commented 2 months ago

Maybe we can start with fixing issue Number 1, i.e. make sure that set_channel_types properly sets the coil_type for eyetracking channels

Sounds good, you up for a PR at some point @scott-huberty ?

scott-huberty commented 2 months ago

Maybe we can start with fixing issue Number 1, i.e. make sure that set_channel_types properly sets the coil_type for eyetracking channels

Sounds good, you up for a PR at some point @scott-huberty ?

Yes, I have a backlog of eye-tracking related issues I need to get to 😅 Feel free to assign this to me. I'll have time in August to submit some PR's.