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

VIZ: EEG sensor locations wrong in inset plots #6304

Closed
drammock opened this issue May 13, 2019 · 11 comments · Fixed by #7066
Closed

VIZ: EEG sensor locations wrong in inset plots #6304

drammock opened this issue May 13, 2019 · 11 comments · Fixed by #7066
Assignees
Milestone

Comments

@drammock
Copy link
Member

EEG sensor plot insets seem to be broken in some cases. Here are a couple examples. First, from plot_compare_evokeds:

eeg

Similar problem with `evoked.plot(spatial_colors=True):

evoked_plot_spatial_colors

Compare those to the sensor positions w/r/t the head on mne.channels.make_eeg_layout(raw.info).plot() (raw.plot_sensors() gives similar result):

sphx_glr_working_with_sensor_locations_003

@massich massich added this to the 0.18 milestone May 15, 2019
@larsoner larsoner removed this from the 0.18 milestone May 16, 2019
@larsoner
Copy link
Member

I don't see this as a blocker for 0.18, which we want to get out tomorrow if possible. So moving the milestone to 0.19.

@larsoner larsoner added this to the 0.19 milestone May 16, 2019
@massich
Copy link
Contributor

massich commented May 17, 2019

Is this related to #5190 ?

@drammock
Copy link
Member Author

@massich I don't know the layout plotting code well enough to say whether they're related. But these examples should make a good test case for whatever fix comes in for #5190 (or vice versa)

@jona-sassenhagen
Copy link
Contributor

Isn't the issue simply that layouts are calculated using both MEG and EEG channels, and EEG channels (in this specific montage) don't go as deep?

In which case this might be working as it should be.

@drammock
Copy link
Member Author

@jona-sassenhagen to me the issue is that the same set of EEG sensors nearly fills the head with raw.plot_sensors() or mne.channels.make_eeg_layout(raw.info).plot() but appear clustered at the crown of the head in the insets in evoked.plot() or plot_compare_evokeds() calls. IMO they should look the same.

@jona-sassenhagen
Copy link
Contributor

Not sure how to deal with that. One issue here is that we make no absolute guarantees here. Consider outlines='skirt'.

Arguably, the behaviour should match, but maybe the behaviour derived from calculating all layouts simultaneously is the better one.

@larsoner
Copy link
Member

Arguably, the behaviour should match, but maybe the behaviour derived from calculating all layouts simultaneously is the better one.

@jona-sassenhagen @drammock can you check my proposal here:

#3987 (comment)

I argue that these plots should always show the sensors reasonably aligned to a ~80cm head. Then it doesn't matter what subselection, etc. you use. For MEG you align using dev_head_t and for EEG you're already in head coords so should be fine. It should be more true to the data and head alignment, and avoid these rescaling issues altogether.

Also related: #5472, #5471

@drammock
Copy link
Member Author

drammock commented Jun 4, 2019

adding another test case for #5472. Would be really nice to be able to do plots like this in the tutorials.

import os
import mne
sample_data_folder = mne.datasets.sample.data_path()
sample_data_raw_file = os.path.join(sample_data_folder, 'MEG', 'sample',
                                    'sample_audvis_raw.fif')
raw = mne.io.read_raw_fif(sample_data_raw_file)
raw.crop(tmax=60).load_data()
raw.plot_sensors(ch_type='eeg', show_names=True)
midline_ch_names = ['EEG {:03}'.format(n) for n in (2, 12, 30, 48, 58, 60)]
midline = raw.copy().pick_channels(midline_ch_names)
midline.plot_sensors(show_names=True)

Figure_1

Figure_2

@agramfort
Copy link
Member

agramfort commented Jun 4, 2019 via email

@larsoner
Copy link
Member

larsoner commented Jun 4, 2019

I started looking at one point and it was a bit of a nightmare. But I'll see if I can look again soon

@larsoner larsoner assigned larsoner and unassigned massich and GuillaumeFavelier Jun 4, 2019
@massich
Copy link
Contributor

massich commented Jun 5, 2019

so did I when I started digitization I expect plot_sensors to get simplified with the refactorings.

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