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

BUG: set_eeg_reference with single channel behavior #12369

Open
larsoner opened this issue Jan 17, 2024 · 13 comments
Open

BUG: set_eeg_reference with single channel behavior #12369

larsoner opened this issue Jan 17, 2024 · 13 comments
Assignees
Milestone

Comments

@larsoner
Copy link
Member

When processing a recent brainvision recording with FCz as the reference (which means it's not in the recording), I wanted to recover this channel in the data, use an average ref proj, and eventually source localize. So I wanted to do:

raw_eeg = mne.io.read_raw_brainvision(eeg_fname).load_data()
raw_eeg.rename_channels(eeg_renames)  # numbers to standard 1020 names
mne.add_reference_channels(raw_eeg, "FCz", copy=False)  # add all-zero channel
raw_eeg.set_montage("standard_1020")  # set locations
raw_eeg.set_eeg_reference(["FCz"])  # set the reference information explicitly
raw_eeg.set_eeg_reference(projection=True)  # switch to an average reference using projection

and have source loc etc. work. However, doing this we end up with two problems for source loc:

  1. raw_eeg.info["custom_ref_applied"] == 1
  2. raw_eeg.info["chs"][ii]["loc"][3:6] is all zeros

Correct behavior here I think would be to have each EEG channel's ch["loc"][3:6] be FCz's ch["loc"][:3], and have custom_ref_applied be set to "off", i.e.:

with raw_eeg.info._unlock():
    for ch in raw_eeg.info["chs"]:
        ch["loc"][3:6] = raw_eeg.info["chs"][-1]["loc"][3:6]
    raw_eeg.info["custom_ref_applied"] = 0

So concretely, I propose that in cases where set_eeg_reference is called with a single channel name and that channel name has a meaningful position (ch["loc"][:3]"), we set custom_ref_applied to False and set ch["loc"][3:6] for all EEG channels properly.

@hoechenberger
Copy link
Member

What is the meaning of custom_ref_applied anyway? What is a "custom" reference?

@larsoner
Copy link
Member Author

At the moment it means "something we can't handle properly in inversion" basically. Things like bipolar refs, different references for different channels, etc. Some of those situations #12366 is meant to handle. Here I'm just concerned with the case that is the standard/current way we can handle things for computing inverses.

@hoechenberger
Copy link
Member

Thanks for the explanation! I like the proposal you made, based on my limited understanding of the issue

@sappelhoff
Copy link
Member

So concretely, I propose that in cases where set_eeg_reference is called with a single channel name and that channel name has a meaningful position (ch["loc"][:3]"), we set custom_ref_applied to False and set ch["loc"][3:6] for all EEG channels properly.

What is stored in the ch["loc"] list?

Other than that, +1 for the proposal.

@larsoner
Copy link
Member Author

For EEG channels, the first three elements are the electrode location and the next three are the reference channel location

@drammock
Copy link
Member

so, to clarify, I've added comments in your example script; are these correct?

raw_eeg = mne.io.read_raw_brainvision(eeg_fname).load_data()
raw_eeg.rename_channels(eeg_renames)  # numbers to standard 1020 names
mne.add_reference_channels(raw_eeg, "FCz", copy=False)  # add all-zero channel
raw_eeg.set_montage("standard_1020")  # set locations
# ↓↓↓↓ this line is where `ch["loc"][3:6]` for all channels would get set to the 
# ↓↓↓↓ location of the reference channel (here FCz)
raw_eeg.set_eeg_reference(["FCz"])  # set the reference information explicitly
# ...and this line ↓↓↓↓↓↓ is where you would set `custom_ref_applied = False`?
raw_eeg.set_eeg_reference(projection=True)  # switch to an average reference using projection

@larsoner
Copy link
Member Author

The custom_ref_applied=False would occur one line sooner, but otherwise correct. As soon as you set a single EEG electrode as the reference point for all other electrodes, you're back to the standard inverse-imaging-able situation. And set_eeg_reference(projection=True) can come later (will actually happen automatically in some cases).

@drammock
Copy link
Member

thanks for the clarification. I mistakenly thought that "custom ref applied" would be False only for average reference --- but I guess it is False for "any case that can safely be converted to average reference with no loss of info"?

@larsoner
Copy link
Member Author

Yeah I think that's right. And FWIW there are probably cases (maybe most of them?) where we could convert to an average ref without losing info or doing anything wrong mathematically, but so far there hasn't been a big need for it so we haven't implemented it. So I think custom_ref_applied means "something other than a single-electrode reference", like bipolar references, multi-electrode references (like using both mastoids), or stuff desired in #12366 .

@larsoner
Copy link
Member Author

larsoner commented Feb 2, 2024

See also #12407 where this same sort of issue was hit by a non-proj average ref.

Concretely I'm thinking we shouldn't set custom_ref_applied in cases where the same reference is applied to all EEG channels. In that case we can apply the average ref proj during inversion and things will be okay. It's cases where users do different references for different channels (like bipolar referencing between some channel pairs) that will be problematic. Stuff like #12369 should be fine. I think we added the custom_ref_applied just to be safe, but I think we can safely relax it in some cases like this.

The one place things will be wrong is in the forward modeling step. Right now we subtract the reference electrode forward from each electrode's forward. But I think it's probably okay/safe enough to set chs[ii]['loc'][3:6] (reference location) to zeros, which in the forward code means "ignore, don't subtract". This is the behavior we've had for standard montages for example.

Then at some point we could add extra information in the extended channel info block about which channels were references (see also #12283) if needed, and fix the forwards as well. But that would be quite far down the road I think.

@nabilalibou
Copy link
Contributor

nabilalibou commented Feb 2, 2024

The custom_ref_applied=False would occur one line sooner, but otherwise correct. As soon as you set a single EEG electrode as the reference point for all other electrodes, you're back to the standard inverse-imaging-able situation. And set_eeg_reference(projection=True) can come later (will actually happen automatically in some cases).

May I ask what you mean by "set_eeg_reference(projection=True)will actually happen automatically in some cases" ?
That Forward/Inverse modeling functions will set custom_ref_applied = False automaticaly under condition at some point ?

@larsoner
Copy link
Member Author

larsoner commented Feb 2, 2024

May I ask what you mean by "set_eeg_reference(projection=True)will actually happen automatically in some cases" ?

I think there are some places in our code where this happens. Can't remember offhand but I think it does... maybe in channel interpolation or something?

That Forward/Inverse modeling functions will set custom_ref_applied = False automaticaly under condition at some point ?

No not this part. If info['custom_ref_applied'] is True it should mean that it's not safe to just add an avg ref proj -- I'm thinking this should indicate some more complicated referencing has been done (like the bipolar case).

@nabilalibou
Copy link
Contributor

nabilalibou commented Feb 2, 2024

I'm thinking this should indicate some more complicated referencing has been done (like the bipolar case)

Yeah could be useful imo, thanks for the clarification !

@larsoner larsoner changed the title BUG: set_eeg_refereence with single channel behavior BUG: set_eeg_reference with single channel behavior Feb 14, 2024
@larsoner larsoner modified the milestones: 1.7, 1.8 Apr 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants