mne-tools / mne-python

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

ylim in plot_evoked_topo #8714

Closed jasmainak closed 3 years ago

jasmainak commented 3 years ago

The ylim parameter in plot_evoked_topo is None by default and for this default, the channels have different ylim. I think this is a bad default because:

1) It does not allow comparison across channels (e.g., lateralization effects) 2) It does not allow you to easily spot bad channels

Instead, the ylim should be determined based on all the channels. One option is to copy the defaults from xplotter which are based on expected physical values of the quantities for evoked responses. The other option is to use the MAD or some such thing so that you get reasonable values even when bad channels are present.

jasmainak commented 3 years ago

Also, not a bug but xplotter has a much smoother workflow since all the channels use a single axis. Thus you can zoom in and out seamlessly instead of having to open and close figures. It really makes a difference for usability

agramfort commented 3 years ago

what makes you think that ylim is not shared across channels from the same type? can you show a screenshot? and show how nicer is looks with xplotter?

jasmainak commented 3 years ago

Try this:

import numpy as np

import mne
from mne.datasets import sample

data_path = sample.data_path()

fname = data_path + '/MEG/sample/sample_audvis-ave.fif'

# Reading evoked data
condition = 'Left Auditory'
evoked = mne.read_evokeds(fname, condition=condition, baseline=(None, 0),
                          proj=False)
evoked.plot_topo()
evoked.plot_topo(ylim=dict(grad=[-0.5, 0.5], mag=[-0.5, 0.5]),
                 scalings=dict(grad=1e13, mag=1e15)) 

you see the bad channel doesn't show in the first plot. What xplotter allows you to do is zoom into say 4 channels by making a square with the mouse and the ylim will automatically adjust. Clicking on individual channels the way it's currently, it's hard to play with the data. I don't yet know how to access xplotter at Martinos, I'll share a screenshot when I can.

agramfort commented 3 years ago

ok so presently you can zoom by drawing a square but the ylim is not adjusted dynamically

that seems something valuable to add !

jasmainak commented 3 years ago

By the way, I hit this ylim issue again today when plotting epochs. If you do:

epochs.plot(scalings='auto') 

it will scale each channel individually. My expectation is that a single value should be chosen because if you do:

epochs.plot(scalings=dict(eeg=20e-6))

it keeps the scale constant across channels. That way you can easily spot bad channels and/or patterns across channels. Not sure what the "channel-wise" scaling is useful for.

ramkpari commented 3 years ago

Working on this!

ramkpari commented 3 years ago

So was working on this issue when I noticed that ylim parameters somehow seem to not apply to all channels in the plot when using plot_evoked_topo . Running the following code sample should be able to showcase what I mean.

import numpy as np

import mne
from mne.datasets import sample

data_path = sample.data_path()

fname = data_path + '/MEG/sample/sample_audvis-ave.fif'

# Reading evoked data
condition = 'Left Auditory'
evoked = mne.read_evokeds(fname, condition=condition, baseline=(None, 0),
                          proj=False)

# Removing non MAG or GRAD type channels 
evoked_meg = evoked.copy().pick_types(meg=True, eeg=False)

# Plotting with all defaults 
evoked_meg.plot_topo() 

# Specifiying different ylims. Notice how the different channels scale for defined ylims in comparison to channel MEG 0113
evoked_meg.plot_topo(ylim=dict(grad=[-0.5, 0.5], mag=[-0.5, 0.5]),scalings=dict(grad=1e13, mag=1e15)) 

evoked_meg.plot_topo(ylim=dict(grad=[-5, 5], mag=[-5, 5]),scalings=dict(grad=1e13, mag=1e15)) 

evoked_meg.plot_topo(ylim=dict(grad=[-15, 15], mag=[-15, 15]),scalings=dict(grad=1e13, mag=1e15))                

I don't know if I've come across an intended feature or if I just stumbled on a different bug while looking at this issue. (@rob-luke @agramfort @drammock @larsoner @sappelhoff )

jasmainak commented 3 years ago

MEG 0113 is a bad channel so it has high amplitudes and I think this is the behavior that should be expected. The previous automatic scaling was not good

ramkpari commented 3 years ago

In this case , I wanted to specifically highlight how channels apart from MEG 0113 don't respond to manually set ylim

jasmainak commented 3 years ago

indeed ... something funky is going on. Do you mind investigating?

ramkpari commented 3 years ago

Yeah , Definitely. For now , it looks like the defined ylims seems to be preserved while within plot_evoked_topo and somehow does some weird stuff after the data gets passed on to class _plot_topo , which is responsible for producing the actual plot.

Also looking back at the original issue, the default ylims being assigned when plot_evoked_topo is called with out any parameters seems to be in the magnitude of several hundreds. For evoked_meg in the above example, it's specifically [(-548.98971293,548.98971293) , (-199.91518048,199.91518048).

jasmainak commented 3 years ago

I see ... put a breakpoint and you'll find out what's going on! Between, I think this issue is in many plotting functions -- so potential for bigger PR after you figure this out

jasmainak commented 3 years ago

@ramkpari can we close this issue now?

ramkpari commented 3 years ago

@ramkpari can we close this issue

Once #9285 is merged it should be good to close.