Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BIDS channel type issue #1046

Closed
arnodelorme opened this issue Aug 7, 2022 · 21 comments · Fixed by #1059
Closed

BIDS channel type issue #1046

arnodelorme opened this issue Aug 7, 2022 · 21 comments · Fixed by #1059
Assignees
Labels

Comments

@arnodelorme
Copy link

arnodelorme commented Aug 7, 2022

bids_path = BIDSPath(root='../ds003061')

Channels 65 to 79 are not EEG (and marked as such in the BIDS repo). Yet MNE thinks 2 of these channels are EEG.

https://openneuro.org/datasets/ds003061

@arnodelorme arnodelorme added the bug label Aug 7, 2022
@jasmainak jasmainak transferred this issue from mne-tools/mne-python Aug 10, 2022
@welcome
Copy link

welcome bot commented Aug 10, 2022

Hello! 👋 Thanks for opening your first issue here! ❤️ We will try to get back to you soon. 🚴🏽‍♂️

@adam2392
Copy link
Member

Hi @arnodelorme

Is this an error when you read the file? BIDSPath is just a class for creating file paths in bids datasets. I'm guessing maybe when you run read_raw_bids(?)

is it possible for you to upload a code segment and also the MNE and related package versions? E.g. a Minimal working example for the error.

output of 'mne sys_info' will show you the relevant versions.

1 similar comment
@adam2392

This comment was marked as duplicate.

@arnodelorme
Copy link
Author

Of course

import os
import os.path as op
import mne
from mne.datasets import sample
from mne_bids import BIDSPath, read_raw_bids, print_dir_tree, make_report, inspect_dataset, mark_channels

bids_path = BIDSPath(root='ds003061', datatype='eeg', suffix = 'eeg')
res = bids_path.match()
rawraw = read_raw_bids(bids_path=res[0], verbose=False)

ds003061 is available from https://openneuro.org/datasets/ds003061

@arnodelorme
Copy link
Author

Sys info


Platform:         macOS-10.16-x86_64-i386-64bit
Python:           3.8.5 (default, Sep  4 2020, 02:22:02)  [Clang 10.0.0 ]
Executable:       /Users/arno/miniconda3/envs/p38env/bin/python
CPU:              i386: 16 cores
Memory:           Unavailable (requires "psutil" package)
mne:              1.1.0
numpy:            1.19.2 {blas=mkl_rt, lapack=mkl_rt}
scipy:            1.4.1
matplotlib:       3.3.3 {backend=module://ipykernel.pylab.backend_inline}

sklearn:          0.24.1
numba:            Not found
nibabel:          Not found
nilearn:          Not found
dipy:             Not found
cupy:             Not found
pandas:           1.2.1
pyvista:          Not found
pyvistaqt:        Not found
ipyvtklink:       Not found
vtk:              Not found
qtpy:             Not found
ipympl:           Not found
pyqtgraph:        Not found
pooch:            v1.6.0

mne_bids:         0.10
mne_nirs:         Not found
mne_features:     Not found
mne_qt_browser:   Not found
mne_connectivity: Not found
mne_icalabel:     Not found   

@hoechenberger
Copy link
Member

hoechenberger commented Aug 10, 2022

@arnodelorme
*_channels.tsv (and _electrodes.tsv) only contain a small subset of the channels present in the data. Hence, I would argue that the dataset is corrupted or incomplete, and the only reason the BIDS validator doesn't complain is because it lacks that capability of actually loading and inspecting the data.

One could argue that MNE-BIDS should handle things more gracefully here (or raise an exception); but ultimately I'd say the dataset needs to be fixed.

and marked as such in the BIDS repo

But where?

And which are the problematic channels, exactly?

Here's a MWE:

# %%

# Data downloaded via:
#     openneuro-py download --dataset ds003061 --include sub-001

import mne_bids

bp = mne_bids.BIDSPath(
    root='/tmp/ds003061',
    subject='001',
    task='P300',
    run='1',
    datatype='eeg',
    suffix='eeg',
    extension='.set',
)
raw = mne_bids.read_raw_bids(bp)

for ch_name in raw.ch_names:
    ch_type = raw.get_channel_types(ch_name)[0]
    ch_idx = raw.ch_names.index(ch_name)
    print(f'{ch_idx:<5} {ch_name:<6} {ch_type}')

yields:

0     Fp1    eeg
1     AF7    eeg
2     AF3    eeg
3     F1     eeg
4     F3     eeg
5     F5     eeg
6     F7     eeg
7     FT7    eeg
8     FC5    eeg
9     FC3    eeg
10    FC1    eeg
11    C1     eeg
12    C3     eeg
13    C5     eeg
14    T7     eeg
15    TP7    eeg
16    CP5    eeg
17    CP3    eeg
18    CP1    eeg
19    P1     eeg
20    P3     eeg
21    P5     eeg
22    P7     eeg
23    P9     eeg
24    PO7    eeg
25    PO3    eeg
26    O1     eeg
27    Iz     eeg
28    Oz     eeg
29    POz    eeg
30    Pz     eeg
31    CPz    eeg
32    Fpz    eeg
33    Fp2    eeg
34    AF8    eeg
35    AF4    eeg
36    AFz    eeg
37    Fz     eeg
38    F2     eeg
39    F4     eeg
40    F6     eeg
41    F8     eeg
42    FT8    eeg
43    FC6    eeg
44    FC4    eeg
45    FC2    eeg
46    FCz    eeg
47    Cz     eeg
48    C2     eeg
49    C4     eeg
50    C6     eeg
51    T8     eeg
52    TP8    eeg
53    CP6    eeg
54    CP4    eeg
55    CP2    eeg
56    P2     eeg
57    P4     eeg
58    P6     eeg
59    P8     eeg
60    P10    eeg
61    PO8    eeg
62    PO4    eeg
63    O2     eeg
64    EXG1   misc
65    EXG2   misc
66    EXG3   misc
67    EXG4   misc
68    EXG5   misc
69    EXG6   misc
70    EXG7   misc
71    EXG8   misc
72    GSR1   eeg
73    GSR2   eeg
74    Erg1   misc
75    Erg2   misc
76    Resp   resp
77    Plet   misc
78    Temp   eeg

@arnodelorme
Copy link
Author

Yes, but this is the actual file. You can see that GSR1 and 2 have the type GSR and that TEMP has the type TEMP which are regular BIDS types.

https://openneuro.org/datasets/ds003061/versions/1.1.1/file-display/sub-001:eeg:sub-001_task-P300_run-1_channels.tsv

name	type	units
Fp1	EEG	microV
AF7	EEG	microV
AF3	EEG	microV
F1	EEG	microV
F3	EEG	microV
F5	EEG	microV
F7	EEG	microV
FT7	EEG	microV
FC5	EEG	microV
FC3	EEG	microV
FC1	EEG	microV
C1	EEG	microV
C3	EEG	microV
C5	EEG	microV
T7	EEG	microV
TP7	EEG	microV
CP5	EEG	microV
CP3	EEG	microV
CP1	EEG	microV
P1	EEG	microV
P3	EEG	microV
P5	EEG	microV
P7	EEG	microV
P9	EEG	microV
PO7	EEG	microV
PO3	EEG	microV
O1	EEG	microV
Iz	EEG	microV
Oz	EEG	microV
POz	EEG	microV
Pz	EEG	microV
CPz	EEG	microV
Fpz	EEG	microV
Fp2	EEG	microV
AF8	EEG	microV
AF4	EEG	microV
AFz	EEG	microV
Fz	EEG	microV
F2	EEG	microV
F4	EEG	microV
F6	EEG	microV
F8	EEG	microV
FT8	EEG	microV
FC6	EEG	microV
FC4	EEG	microV
FC2	EEG	microV
FCz	EEG	microV
Cz	EEG	microV
C2	EEG	microV
C4	EEG	microV
C6	EEG	microV
T8	EEG	microV
TP8	EEG	microV
CP6	EEG	microV
CP4	EEG	microV
CP2	EEG	microV
P2	EEG	microV
P4	EEG	microV
P6	EEG	microV
P8	EEG	microV
P10	EEG	microV
PO8	EEG	microV
PO4	EEG	microV
O2	EEG	microV
EXG1	MISC	n/a
EXG2	MISC	n/a
EXG3	MISC	n/a
EXG4	MISC	n/a
EXG5	MISC	n/a
EXG6	MISC	n/a
EXG7	MISC	n/a
EXG8	MISC	n/a
GSR1	GSR	n/a
GSR2	GSR	n/a
Erg1	MISC	n/a
Erg2	MISC	n/a
Resp	RESP	n/a
Plet	MISC	n/a
Temp	TEMP	n/a

@hoechenberger
Copy link
Member

Yes, but this is the actual file. You can see that GSR1 and 2 have the type GSR and that TEMP has the type TEMP which are regular BIDS types.

https://openneuro.org/datasets/ds003061/versions/1.1.1/file-display/sub-001:eeg:sub-001_task-P300_run-1_channels.tsv

Oh, right.

I only looked at the TSV in my web browser and didn't realize there was a scrollbar on the right, so I only saw the first few channels 🤦

@hoechenberger
Copy link
Member

We currently use the following channel type mapping from BIDS to MNE:

elif fro == 'bids' and to == 'mne':
mapping = dict(EEG='eeg', MISC='misc', TRIG='stim', EMG='emg',
ECOG='ecog', SEEG='seeg', EOG='eog', ECG='ecg',
RESP='resp', NIRS='fnirs_cw_amplitude',
# No MEG channels for now
# Many to one mapping
VEOG='eog', HEOG='eog', DBS='dbs')

We need to add TEMP and GSR here.

I don't believe MNE has "proper" support for these, so we should map them to misc.
@larsoner @agramfort WDYT?

@sappelhoff I don't think we should silently apply the BIDS datatype's channel type to channels for which we don't have a proper BIDS -> MNE mapping defined; instead, we should default to misc. WDYT?

@hoechenberger
Copy link
Member

RESP='resp',

Either this is a bug, or we have incorrect documentation in MNE – RESP is not "respiration"

https://github.com/mne-tools/mne-python/blob/8baaa7675ea34d3b98dd2875ad8b5efaf6d44d16/mne/io/pick.py#L400-L402

We'd need to check the FIFF specs.

@hoechenberger
Copy link
Member

… says "respiration monitoring" here:

https://github.com/mne-tools/fiff-constants/blob/30d89d420d50c9e465f46524d4901795dac66741/DictionaryTypes.txt#L257

@larsoner What's correct now..? Is "resp" for "response" or for "respiration"?

@agramfort
Copy link
Member

for some reason my answer from yesterday never appeared here. Let me paste it here:

this data contains channels with type "temp" and "GSR"

I don't think its part of the standard. Cf https://bids-specification.readthedocs.io/en/stable/04-modality-specific-files/03-electroencephalography.html#channels-description-_channelstsv

Type of channel; MUST use the channel types listed below. Note that the type MUST be in upper-case.

Must be one of: "MEGMAG", "MEGGRADAXIAL", "MEGGRADPLANAR", "MEGREFMAG", "MEGREFGRADAXIAL", "MEGREFGRADPLANAR", "MEGOTHER", "EEG", "ECOG", "SEEG", "DBS", "VEOG", "HEOG", "EOG", "ECG", "EMG", "TRIG", "AUDIO", "PD", "EYEGAZE", "PUPIL", "MISC", "SYSCLOCK", "ADC", "DAC", "HLU", "FITERR", "OTHER".

mne_bids converts them to EEG when it does not understand the type. You should get warnings when attempting to read the files with:

from mne_bids import BIDSPath, read_raw_bids

bp = BIDSPath(
    root='./',
    subject='001',
    task='P300',
    run='1',
    datatype='eeg',
    suffix='eeg',
    extension='.set',
)

raw = read_raw_bids(bp)

@hoechenberger
Copy link
Member

I don't think its part of the standard.

It is, you need to scroll down a little:

image

@larsoner
Copy link
Member

@larsoner What's correct now..? Is "resp" for "response" or for "respiration"?

the fif-constants should be the gold standard, and it's "respiration" there, too:

https://github.com/mne-tools/fiff-constants/blob/aae5960007ee8a67dfc07535ea37d421d37dfe1b/DictionaryTypes.txt#L257

So our docs are wrong, can you make a quick PR to fix them?

@hoechenberger
Copy link
Member

hoechenberger commented Aug 15, 2022

Thanks @larsoner! I already fixed this a few days ago, I don't have the PR id at und right now though, as I'm on my phone

@larsoner
Copy link
Member

Okay great!

@agramfort
Copy link
Member

agramfort commented Aug 16, 2022 via email

@hoechenberger
Copy link
Member

@arnodelorme
With #1052, we now set the type of all "unknown" and yet-unsupported BIDS channel types to "misc" when reading the data with MNE-BIDS.

This ensures that the temperature and GSR channels in your dataset will not show up as "eeg" channels anymore (but as "misc").

The next step for us is to amend the FIFF standard and add a proper EDA/GSR and temperature channel type. We're tracking this at mne-tools/mne-python#11055

@arnodelorme
Copy link
Author

arnodelorme commented Aug 19, 2022 via email

hoechenberger added a commit to hoechenberger/mne-bids that referenced this issue Aug 28, 2022
Also updates tiny_bids

Will fail until mne-tools/mne-python#11108 has been merged

Fixes mne-tools#1046
agramfort pushed a commit that referenced this issue Aug 31, 2022
* Add support for GSR and temperature channels

Also updates tiny_bids

Will fail until mne-tools/mne-python#11108 has been merged

Fixes #1046

* Indent

* Preserve info

* Require MNE devel to run tests

* Fix unit writing

* Write correct BIDS temperature unit (oC)

* Better code

* Add conditional

* No warning

* Skip test on MNE stable

* Decorated the wrong test

* Add channel statuses

* Update doc/whats_new.rst

Co-authored-by: Stefan Appelhoff <stefan.appelhoff@mailbox.org>

* Fix channel description

* Manually curate tiny_bids to keep the diff small

* Revert changes related to _orig_units handling

* Fix test

Co-authored-by: Stefan Appelhoff <stefan.appelhoff@mailbox.org>
@hoechenberger
Copy link
Member

hoechenberger commented Aug 31, 2022

@arnodelorme Temperature and GSR channels should now work as expected when combining the development versions of MNE-Python and MNE-BIDS.

Well, not quite, as we still have an issue with plotting:

mne-tools/mne-python#11112

But loading and processing the data should all be good!

Thank you for reporting this issue!

@arnodelorme
Copy link
Author

arnodelorme commented Oct 11, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
5 participants