RFC, WIP: Add eeg_reference kwarg to write_raw_bids()

[hoechenberger]

PR Description

This PR adds an eeg_reference kwarg to write_raw_bids(), allowing users to set EEGReference (EEG) and iEEGReference (iEEG).

Con:

• growing API
• where do we stop? there's like a million optional params for different recording types; we cannot add all of them as kwargs

Pro:

• an EEG reference is such a fundamental property of a recording, it must be extremely convenient to add it to the data in a single go without having to manually fiddle with the JSON sidecars, as is currently suggested in the docs:

  # Now it's time to manually check the BIDS directory and the meta files to add
  # all the information that MNE-BIDS could not infer. For instance, you must
  # describe EEGReference and EEGGround yourself. It's easy to find these by
  # searching for "n/a" in the sidecar files.
  

We need a general discussion about this at one point; however I am deeply convinced that for now, having an eeg_reference param in write_raw_bids() is the best option. But you're free to try and change my mind ;)

😅

Merge checklist

Maintainer, please confirm the following before merging:

• All comments resolved
• This is not your own PR
• All CIs are happy
• PR title starts with [MRG]
• whats_new.rst is updated
• PR description includes phrase "closes <#issue-number>"

[sappelhoff]

agreed with your pro and con, given that EEGReference is required

[hoechenberger]

given that EEGReference is required

I should point out that we always wrote it in the past, but set it to 'n/a' – kind of a dirty workaround to produce BIDS-compliant datasets

[agramfort]

what bothers me is that the raw.info should contain this information and in an ideal world we should not need to add any parameter to write_raw_bids...

can we start by seeing how to write properly mne data on which set_eeg_reference has been applied?

[hoechenberger]

can we start by seeing how to write properly mne data on which set_eeg_reference has been applied?

Yes we could do that. This was also my very first idea. I ditched / postponed it because that's typically not how things work in EEG:

At least with the systems I've worked with in the past, the recorded data would typically not contain the reference electrode by default – it would simply be missing. The signals of other electrodes, however, would readily be referenced to the (missing) reference.

Now if you were to write these data to BIDS immediately, you'd need to have a way to specify what the reference was even though it's now missing.

---

In MNE, I'd typically add this channel back with all zeros via add_reference_channels(). Question is whether this would already be a derivative.

[jasmainak]

I can live with it but not super enthusiastic

Instead I'd suggest having an API to find the 'n/a' entries in a dataset, return them as a dict and update them across the dataset. Didn't we have an issue somewhere? We can't have a param for every entry in the json files ...

[jasmainak]

na_entries = find_entities(bids_root, cond='n/a')
na_entries.update(eeg_reference=eeg_reference)
save_entities(bids_root, na_entries)

[adam2392]

Could use this function:

mne-bids/mne_bids/sidecar_updates.py

Line 16 in 35f3c3d

def update_sidecar_json(bids_path, entries, verbose=True):

[jasmainak]

nice @adam2392 you are the best!

[hoechenberger]

Yes, I'm aware of this function and it's already mentioned in the documentation.

My opinion is that for this specific use case – adding an EEG reference – users shouldn't have to do additional work after calling write_raw_bids() – partly because it's inconvenient, partly because some will simply forget about it (?)

Given there's such a plethora of potential parameters – and we cannot expect users to read the BIDS specification! – I'm wondering whether it could also make sense to have some kind of functionality that specifically helps with adding the REQUIRED and RECOMMENDED fields.

Look at the specs for *_eeg.json, for example:
https://bids-specification.readthedocs.io/en/stable/04-modality-specific-files/03-electroencephalography.html#sidecar-json-_eegjson

This list is sheer endless, and currently MNE-BIDS only fills out a few RECOMMENDED fields – and some of the REQUIRED fields are simply set to 'n/a' to make the validator pass. That's not how things should be. And like I said, we shouldn't expect users to read the specs, esp. since they can change from revision to revision. We need to give users a hand.

I think this EEGLAB tool does a way better job at guiding users than we do:
https://github.com/sccn/bids-matlab-tools/wiki#export-datasets-to-bids-from-the-graphic-interface

I wonder if we need something similar to make all those BIDS options really accessible to our users.

[jasmainak]

okay fair enough. But is EEG reference special in any way from the other parameters? I totally agree that we need to hold the user's hand as much as we can. Would it make sense to:

1. Remove writing 'n/a' to the fields to hack the passing of validator?
2. Then add a function ... or maybe a new BIDS object (like the Info object in MNE) that prepopulates the fields? E.g., if you do:

BIDSTemplate(data_type='eeg')

you will get a dict with empty fields. But if you do:

BIDSTemplate(bids_root)

you will get a dict with fields populated with what is in the dataset. And then you can fix the examples so they explicitly update the required fields

[hoechenberger]

you will get a dict with fields populated with what is in the dataset. And then you can fix the examples so they explicitly update the required fields

This is getting a little more complex if we consider that each participant and session can have different settings… Also it would require you to already have a BIDS dataset.

[adam2392]

Since (i)EEGReference is a REQUIRED field, then it makes sense to allow that to be passed to write_raw_bids, so I am more agreeing with @hoechenberger now... Are there many more REQUIRED fields that we aren't explicitly writing?

Other ideas

For more advanced usage, would we one day want to allow just **kwargs arguments to write_raw_bids, where a user can pass ANY REQUIRED fields to overwrite?

Another idea that is probably unwieldy but will throw out there :p, could we create a dictionary template with the default values for all the REQUIRED and RECOMMENDED fields for MEG/EEG/iEEG and have the user import them, set certain fields themselves and then pass to write_raw_bids?

[jasmainak]

Question is whether this would already be a derivative.

The definition of derivatives is somewhat based on use-case. Is there a case where the user may need to go to the previous data and change the processing for some reason? If the answer is no, you can distribute it as source data IMO :)

[hoechenberger]

I have decided that the best solution would be to close this PR and instead amend our documentation, suggesting users do the following before calling write_raw_bids:

• if the reference channels are present in the data, call raw.set_eeg_reference() to make the reference explicitly known
• if the reference channels are not present in the data:
  • add them via raw.add_reference_channels()
  • call raw.set_eeg_reference() to make the reference explicit

The cool thing would be that if the reference electrode is part of the montage, we'd get the reference electrode location for free :)

Then call write_raw_bids(), which should extract the reference channel info from raw.info['chs'] -> this will need a PR
Easiest is to start with the assertion that all channels have the same reference, so we can write it to [i]EEGReference in the sidecar JSON.

[hoechenberger]

Since we're still blocked by upstream limitations (MNE doesn't keep track of which custom reference was applied), I'm inclined to revive this PR...

[larsoner]

MNE doesn't keep track of which custom reference was applied

In principle this should be fixable. info['chs'][ii]['loc'][3:6] is supposed to store the position of the reference electrode. So that could be checed against the info['chs'][jj]['loc'][0:3] (or info['dig']) to figure out which electrode in the dataset is the reference. This will be a decent amount of effort to get right but I think it would work.

[hoechenberger]

Yes, I believe this was the approach you proposed a couple of years back, but nobody has gotten around implementing it yet. So I was wondering if it could make sense to move forward here (MNE-BIDS) to provide a solution, even if it's not the "best"