Skip to content

MRG, BUG: Fix STC limit bug#8202

Merged
GuillaumeFavelier merged 1 commit intomne-tools:masterfrom
larsoner:stc
Sep 3, 2020
Merged

MRG, BUG: Fix STC limit bug#8202
GuillaumeFavelier merged 1 commit intomne-tools:masterfrom
larsoner:stc

Conversation

@larsoner
Copy link
Copy Markdown
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:

Details 
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
Copy Markdown
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.

GuillaumeFavelier
GuillaumeFavelier approved these changes Sep 3, 2020
View reviewed changes
Copy link
Copy Markdown
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
Copy Markdown
Contributor

GuillaumeFavelier commented Sep 3, 2020

Thanks @larsoner

@GuillaumeFavelier GuillaumeFavelier mentioned this pull request Sep 3, 2020
Merged
@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
@larsoner
Merge remote-tracking branch 'upstream/master' into fixref
ed87713 
* 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
@larsoner @marsipu
BUG: Fix STC limit bug (mne-tools#8202)
4d1c398
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@agramfort agramfort agramfort approved these changes

+1 more reviewer

@GuillaumeFavelier GuillaumeFavelier GuillaumeFavelier approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

0.21

Development

Successfully merging this pull request may close these issues.

BUG: inconsistent auto-clim handling for SourceEstimate and VectorSourceEstimate

3 participants

@larsoner @GuillaumeFavelier @agramfort