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

MRG, BUG: Fix STC limit bug #8202

Merged
merged 1 commit into from
Sep 3, 2020
Merged

Conversation

larsoner
Copy link
Member

@larsoner larsoner commented Sep 2, 2020

Close #7911

On master, auto control points for the vec STC are [1.86911522 2.26944519 6.37046324] and standard STC are [ 3.95048065 4.56941314 17.72451438]. On this PR, there are two fixes:

  1. vector_alpha actually has an effect
  2. _process_clim uses the .magnitude() data for vector mode, which makes more sense.

On this PR, this code:

import os.path as op
import mne

data_path = mne.datasets.sample.data_path()
sample_dir = op.join(data_path, 'MEG', 'sample')
subjects_dir = op.join(data_path, 'subjects')
inv = mne.minimum_norm.read_inverse_operator(op.join(
    sample_dir, 'sample_audvis-meg-oct-6-meg-inv.fif'))
evoked = mne.read_evokeds(op.join(sample_dir, 'sample_audvis-ave.fif'))[0]
evoked.apply_baseline((None, 0))
stc = mne.minimum_norm.apply_inverse(
    evoked, inv, method='dSPM', verbose='debug', pick_ori='vector')
initial_time = 0.1
# stc, kwargs = stc, dict(overlay_alpha=0.5, brain_alpha=0.5, vector_alpha=0.)
stc, kwargs = stc.magnitude(), dict(alpha=0.5, surface='white')
brain = stc.plot(subjects_dir=subjects_dir, initial_time=initial_time,
                 clim='auto', hemi='lh', views='lat',
                 smoothing_steps='nearest', verbose=True,
                 time_viewer=False, size=(600, 600), background='k',
                 **kwargs)
brain.show_view('lat')
  • Run as is (magnitude mode):
    • Mayavi
      Screenshot from 2020-09-02 15-47-59
    • PyVista
      Screenshot from 2020-09-02 15-48-25
  • Run in vector mode (second stc, kwargs line commented out) produces:
    • Mayavi
      Screenshot from 2020-09-02 15-49-21
    • PyVista
      Screenshot from 2020-09-02 15-49-44

The slight coloration of the brain itself differs because mayavi has backface culling on for the surface and our _Brain doesn't. I actually think our behavior is probably better for translucent brains.

There is a bug/mismatch where the limits are not computed properly in auto mode, probably related to whether or not the entire stc is used, or the initial time.

@larsoner larsoner added this to the 0.21 milestone Sep 2, 2020
@larsoner
Copy link
Member Author

larsoner commented Sep 2, 2020

@GuillaumeFavelier feel free to merge if you're happy, failure is just the 3.8 segfault bug. Also feel free to look into that one if you want, not sure why it started happening.

Copy link
Contributor

@GuillaumeFavelier GuillaumeFavelier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@GuillaumeFavelier GuillaumeFavelier merged commit 9749836 into mne-tools:master Sep 3, 2020
@GuillaumeFavelier
Copy link
Contributor

Thanks @larsoner

@larsoner larsoner deleted the stc branch September 3, 2020 12:52
larsoner added a commit to libertyh/mne-python that referenced this pull request Sep 9, 2020
* upstream/master: (489 commits)
  MRG, DOC: Fix ICA docstring, add whitening (mne-tools#8227)
  MRG: Extract measurement date and age for NIRX files (mne-tools#7891)
  Nihon Kohden EEG file reader WIP (mne-tools#6017)
  BUG: Fix scaling for src_mri_t in coreg (mne-tools#8223)
  MRG: Set pyvista as default 3d backend (mne-tools#8220)
  MRG: Recreate our helmet graphic (mne-tools#8116)
  [MRG] Adding get_montage for montage to BaseRaw objects (mne-tools#7667)
  ENH: Allow setting tqdm backend (mne-tools#8177)
  [MRG, IO] Persyst reader into Raw object (mne-tools#8176)
  MRG, BUG: Fix errors in IO/loading/projectors (mne-tools#8210)
  MAINT: vectorize _read_annotations_edf (mne-tools#8214)
  FIX : events_from_annotation when annotations.orig_time is None and f… (mne-tools#8209)
  FIX: do not project to sphere; DOC - explain how to get EEGLAB-like topoplots (mne-tools#7455)
  [MRG, DOC] Added linear algebra of transform to doc (mne-tools#7087)
  FIX: Travis failure on python3.8.1 (mne-tools#8207)
  BF: String formatting in exception message (mne-tools#8206)
  BUG: Fix STC limit bug (mne-tools#8202)
  MRG, DOC: fix ica tutorial (mne-tools#8175)
  CSP component order selection (mne-tools#8151)
  MRG, ENH: Add on_missing to plot_events (mne-tools#8198)
  ...
marsipu pushed a commit to marsipu/mne-python that referenced this pull request Oct 14, 2020
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

Successfully merging this pull request may close these issues.

BUG: inconsistent auto-clim handling for SourceEstimate and VectorSourceEstimate
3 participants