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

Write out combined EEG and iEEG recordings #752

Closed
richardkoehler opened this issue Mar 25, 2021 · 14 comments · Fixed by #774
Closed

Write out combined EEG and iEEG recordings #752

richardkoehler opened this issue Mar 25, 2021 · 14 comments · Fixed by #774

Comments

@richardkoehler
Copy link
Contributor

Describe the problem

Currently, as far as I know, it is not possible to write out a file when there are both EEG and iEEG channels in the recording. MNE-BIDS throws an error and so I have to set EEG channels to "misc" to be able to still write the files. ("ValueError: Both EEG and iEEG channels found in data. There is currently no specification on how to handle this data. Please proceed manually.")
I know that this has probably been discussed before and that the issue likely stems from the fact that multimodal recordings are not clearly defined in BIDS, but I don't really see a reason why it shouldn't be possible to store multimodal data in the same file.

Describe your solution

Maybe it would be possible to let the user decide by setting the parameter datatype to either eeg or ieeg.
If this is not set, we could infer the datatype from the more prevalent channel type.

@jasmainak
Copy link
Member

How would you store the raw file? Do you want to store it in EEG folder or iEEG or split it into the two folders?

@richardkoehler
Copy link
Contributor Author

We simultaneously record subcortical LFP and cortical EEG signals (same front end, same processor) and so the data should definitely be stored in the same file. I would like to store it all in an iEEG folder, as the iEEG data is the more important one for us. What do you think? Currently we are indirectly being forced to either split the data from the same recording into two files (which itself is already kind of a violation of the BIDS standard for raw data right?), or to assign a false channel type to either the EEG or the iEEG channels.

@jasmainak
Copy link
Member

I seem to remember a discussion on the bids-specification github. You might want to look into it ... it might have some hints what is the consensus etc.

@richardkoehler
Copy link
Contributor Author

@jasmainak Thanks for the link! This discussion super interesting and relevant for us, because we are also simultaneously recording MEG and iEEG data, which are then stored in separate files and the "SimultaneousRecording" parameter could tell us which files were recorded at the same time. However, the issue I was describing ist somewhat different, because in my case, the EEG and iEEG data is recorded with the same hardware and stored in exactly the same file. And the consensus in the issue you posted seems to be that these should be stored in the same data file and then the sidecar files (e.g. "_ieeg.json") will tell you whether there are EEG and iEEG channels in the same file. So I think there should definitely be an option to store EEG and iEEG data in the same file, if they were recorded together, but maybe I am mistaken.

@jasmainak
Copy link
Member

I see! I just wanted to share the pointer as you might find the answer what's the right thing to do. I agree that it sounds like we need a PR to mne-bids. Do you want to contribute? I'm sure @hoechenberger or @adam2392 or @sappelhoff might be able to help if you can't!

@sappelhoff
Copy link
Member

I think I would just handle this as a pure ieeg dataset that just happens to have some EEG channels (these are actually ECOG channels if I get it correctly), so +1 to extend mne-bids here.

@richardkoehler
Copy link
Contributor Author

@sappelhoff Actually no these are really EEG channels :) ECOG channels don't pose a problem, as they are also iEEG channels (like LFP/sEEG/DBS channels). But yes, I definitely share your opinion that the iEEG channels (ECOG or LFP/sEEG/DBS) are more important, so I would agree in treating this like an iEEG dataset - that happen to have additional EEG channels.
@jasmainak I would be very happy to contribute, but I might need some guidance as I'm not familiar with the details of what is happening under the MNE-BIDS hood. So when calling write_raw_bids, MNE-BIDS checks what types of channels are inside the recording and then infers the type of dataset right? If we detect both EEG and anyone of LFP/DBS/SEEG channel types, we would treat the dataset like iEEG data and set the datatype to "ieeg" right? I think this is the only case I ran into an Error with.

@sappelhoff
Copy link
Member

cortical EEG signals

just to clarify for me: did you record EEG on the scalp, or directly on the brain / below the skull? :-D

@richardkoehler
Copy link
Contributor Author

@sappelhoff Haha, yes we record EEG on the scalp and simultaneously LFP (DBS) signals. But you are right that sometimes we also record ECOG (subdural) additionally, so then we have LFP + ECOG + scalp EEG in the same recording.

@jasmainak
Copy link
Member

@richardkoehler the automatic inference is an artifact of the time BIDSPath didn't exist. I think it needs to be made smarter now. My sense is that you _handle_dataype should be more like _check_datatype taking datatype as an argument and throwing an error if the datatype specified by the user doesn't exist in the data. Otherwise, the user should be trusted to know what they are doing if there are multiple data types and no automatic guessing done. Sounds reasonable @hoechenberger ?

@richardkoehler
Copy link
Contributor Author

@jasmainak Okay that's super interesting to know. Then I would say it is definitely time to change this now :) I also agree with the notion that the datatype should be specified by the user and no assumptions should be made by MNE-BIDS.

@jasmainak
Copy link
Member

Give it a shot and we'll review it :)

@richardkoehler
Copy link
Contributor Author

@jasmainak @hoechenberger
This is related to my PR #774
My code is probably not without flaws, but I am realizing now that a lot of people have probably relied on the automatic data type inferring in write_raw_bids, which is why a lot of tests will now fail. I even "reinstated" automatic data type inferring for data that contains "meg" channels. But I think that many tests are still failing because we now check if the datatype in the BIDSPath object actually exists in the data. Of course, this check totally makes sense but we will have to adapt a lot of tests, and if we actually remove the automatic datatype inferring, we should keep in mind that this will "break" existing code for a lot of people out there, that have been "relying" on this in the past. I would love to hear your thoughts on this.

@richardkoehler
Copy link
Contributor Author

closed by #774

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

Successfully merging a pull request may close this issue.

4 participants