mne-tools / mne-python

MNE: Magnetoencephalography (MEG) and Electroencephalography (EEG) in Python
https://mne.tools
BSD 3-Clause "New" or "Revised" License
2.7k stars 1.31k forks source link

question: puzzling results in epochs.plot_topo_image #6112

Closed drammock closed 5 years ago

drammock commented 5 years ago

Setup:

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, preload=True, verbose=False)
events = mne.find_events(raw, stim_channel='STI 014')
epochs = mne.Epochs(raw, events, preload=True)

I expected these two options to yield the same results, but they're very different:

# OPTION 1
epochs.copy().pick_types(meg=False, eeg=True).plot_topo_image()

Figure_1

# OPTION 2
eeg_layout = mne.channels.find_layout(raw.info, ch_type='eeg')                                                            
epochs.plot_topo_image(layout=eeg_layout)

Figure_2

Is the difference here intentional, or a bug?

larsoner commented 5 years ago

Looks like a scaling / vmiv / vmax problem in the first example

drammock commented 5 years ago

So, not intentional I take it. I'll have a look.

jona-sassenhagen commented 5 years ago

For the purpose of DRY, this could default to #6078 ...

drammock commented 5 years ago

@jona-sassenhagen it looks like _plot_masked_image (and its proposed public version) require/create an axes object each time called, whereas plot_topo_image_epochs does the _iter_topography thing to plot each little imagmap inside one big axes. So changing plot_topo_image_epochs to use _plot_masked_image behind the scenes might take some heavy lifting.

(as an aside, I noticed that _plot_masked_image has a picks argument that appears to go unused?)

jona-sassenhagen commented 5 years ago

Hm? iter_topography creates one axis per sensor, right?

https://github.com/mne-tools/mne-python/blob/9b5ae5104ecdeaea13a88273c712382bf131162c/mne/viz/topo.py#L214

So this would essentially be about changing the show_func mostly, right? Not saying it is trivial.

Either way, the same applies to https://github.com/mne-tools/mne-python/issues/4746, which I will do during the sprint.

drammock commented 5 years ago

ah, you're right @jona-sassenhagen. I thought it was passing unified=True by default, but that must have been something else I was looking at yesterday.

drammock commented 5 years ago

@larsoner I'm convinced that in fact both of the plots above have messed up vmin/vmax. Compare the detail plots:

epochs.plot_image(picks='EEG 060') 

Figure_3

versus clicking on that channel after

epochs.copy().pick_types(meg=False, eeg=True).plot_topo_image()

Figure_5

versus clicking on that channel after

epochs.plot_topo_image(layout=eeg_layout)

Figure_4

Still working out how to correct it.

jona-sassenhagen commented 5 years ago

I remember fixing a lot of code in epochs.plot_image to get the vlims right. Maybe check there.

Or I could indeed try to hook this up to _plot_masked_image at the sprint, so it could simply share that vlim functionality.

drammock commented 5 years ago

I'm gonna punt on this one until the sprint, I think. I put in a draft PR for what I think should be happening, but the resulting plots don't look right (too much of the signal lands near zero => images are mostly grey/white). I also considered an approach using np.quantile to set vmin/vmax at the (0.01, 0.99) quantiles to pull some color into the middle range... maybe worth exploring?

mmagnuski commented 5 years ago

I've had this problem too some time ago, but now I rarely use plot_topo_image.

jona-sassenhagen commented 5 years ago

Mayyyyybe with #6186 in we can use _set_image_lims_and_scale for this.