MRG: Add initial arguments to vol viz #6046

larsoner · 2019-03-11T23:18:03Z

When using VolSourceEstimate.plot() I found myself wanting a way to specify the initial time and initial position. This PR thus:

Adds support for initial_time (like in surf source est plotting)
Adds support for initial_position
Adds support for passing a morph instead of SourceSpaces as src (see Notes for details/rationale)
Fixes indexing of morphed STC (see example)
Speeds up tests a little bit

My particular use case involved replicating a figure someone sent me with initial_time=0.2 and initial_pos=(-0.04, -0.034, -0.002) and now I can script it to give:

Not ready for merge because there is some bug with indexing the morphed STC.

codecov · 2019-03-12T00:11:01Z

Codecov Report

❗ No coverage uploaded for pull request base (master@9585e93). Click here to learn what that means.
The diff coverage is 93.4%.

@@            Coverage Diff            @@
##             master    #6046   +/-   ##
=========================================
  Coverage          ?   89.62%           
=========================================
  Files             ?      421           
  Lines             ?    76110           
  Branches          ?    12441           
=========================================
  Hits              ?    68211           
  Misses            ?     5100           
  Partials          ?     2799

mne/viz/tests/test_3d.py

jasmainak · 2019-03-12T22:05:15Z

@larsoner what is the image we are looking at? Is it the max? I remember being confused by this in the beginning. If you click on a certain slice that looks like the max, it does not necessarily give you the max in the time course. Because you are looking at the time course for that particular slice, not the time course for the slice corresponding to the global max. How does the image look when you click at t = 0.16?

larsoner · 2019-03-12T22:15:58Z

Try the example here morphed to fsaverage in either mode, clicking and the maps/time course should be broken

examples/inverse/plot_lcmv_beamformer_volume.py

larsoner · 2019-03-17T01:52:58Z

@jasmainak why in glass_brain mode are images resampled to the bg_img? I don't understand this bit. (For now I've removed the code for it since it did not seem to be useful.) I don't even see how bg_img would be useful, since in the plot_glass_brain docs it says that the image should be in MNI coords, but we do nothing to guarantee this.

We might want to use the given subject's talxfm to automagically transform the image, or -- probably safer -- add a big warning (with instructions for people on how to do the resampling themselves).

This doesn't really tell us why the morph to fsaverage clicking does not work properly, though, since fsaverage is in MNI space already... more work to be done there still. But at least the non-morphed versions seem to work well with the given changes (which mostly simplify things).

jasmainak · 2019-03-17T07:18:53Z

@jasmainak why in glass_brain mode are images resampled to the bg_img? I don't understand this bit. (For now I've removed the code for it since it did not seem to be useful.) I don't even see how bg_img would be useful, since in the plot_glass_brain docs it says that the image should be in MNI coords, but we do nothing to guarantee this.

sorry for this. I will need to dig up a bit to figure out what was the exact reasoning at the time. But from what I seem to remember, the stc image was not in the same coordinate space as the T1, which is why you need to do this. Once you are in the same space as T1, then plot_stat_map and plot_glass_brain become equivalent.

We might want to use the given subject's talxfm to automagically transform the image, or -- probably safer -- add a big warning (with instructions for people on how to do the resampling themselves).

sure. I am always confused about the different coordinate systems. From what I can recall there was an interplay between 3 coordinate systems -- the one that the STC was in, then the T1 and then the one that nilearn expects the images to be in -- which is MNI? So, the talxfm is the transform that takes you from T1 to MNI, and is stored in the metadata of the T1 image?

britta-wstnr · 2019-04-11T14:34:03Z

@jasmainak and @larsoner I went through quite some transformation matrix and coordinate system fun lately and would be happy to share my 2 cents at the sprint if desired? I also wrote a little snippet to add markers to volume plots (e.g., for marking simulation voxels etc.; however, with plot_stat_map), could be nice to have here, too?

larsoner · 2019-04-11T14:36:05Z

I went through quite some transformation matrix and coordinate system fun lately and would be happy to share my 2 cents at the sprint if desired

Yes let's figure this out at the sprint after we complete The Great Beamformer Unification of 2019™

lgtm-com · 2019-04-27T05:33:32Z

This pull request introduces 1 alert when merging e7788e5 into 5d80e26 - view on LGTM.com

new alerts:

1 for Unused import

Comment posted by LGTM.com

lgtm-com · 2019-09-05T17:23:15Z

This pull request introduces 1 alert and fixes 1 when merging 0a44aa8 into d1730ca - view on LGTM.com

new alerts:

1 for Unused import

fixed alerts:

1 for Wrong name for an argument in a call

lgtm-com · 2019-09-05T21:30:50Z

This pull request fixes 1 alert when merging 998e936 into 63e9ad2 - view on LGTM.com

fixed alerts:

1 for Wrong name for an argument in a call

examples/inverse/plot_lcmv_beamformer_volume.py

larsoner · 2019-09-11T18:24:32Z

This monster is finally done. There were several bugs (see changes/latest.inc), but to summarize:

glass_brain plotting assumes your data are in MNI coords, otherwise the overlay is wrong. So we modify img.affine to add an extra transformation to those coords.
stc_morph = SourceMorph.apply(stc) had stc_morph.vertices defined improperly (assuming Z, then Y, then X changed most rapidly, i.e. 'C' order, but the MNE-C code defines it using Fortran order)
glass_brain MIP picking function needed to make use of the voxel data orientation, which for fsaverage's T1 is LIA for example, to know which i/j/k dimension to look into, set, and display

Ready for review/merge from my end. Modified example showing correct plots for different combinations of stat_map, glass_brain, and passing morph:

https://15223-1301584-gh.circle-artifacts.com/0/dev/auto_examples/inverse/plot_lcmv_beamformer_volume.html#sphx-glr-auto-examples-inverse-plot-lcmv-beamformer-volume-py

lgtm-com · 2019-09-11T18:42:47Z

This pull request introduces 3 alerts and fixes 1 when merging 773566a into 13c8e69 - view on LGTM.com

new alerts:

1 for Unused import
1 for Unreachable code
1 for Variable defined multiple times

fixed alerts:

1 for Wrong name for an argument in a call

agramfort

maybe @jasmainak can have a look or test to confirm it works as expected? thanks !

larsoner · 2019-09-12T20:47:39Z

Glass brain is almost "on fire". Not sure if it's this PR or this was always the case.

MIP can look that way when a lot of the brain is active. Center on the N100 peak and it looks more like what you're probably expecting.

do you think we should fix the ylim for the time series? It would be a bit misleading otherwise. Might be easy to include in this PR while you are already at it.

I can try some heuristic like 0.5 and 99.5th percentile of the data or something. I'll try some stuff and see if I can come up with something reasonable. Eventually we might want a time_ylim or something to allow fixing it manually

larsoner · 2019-09-13T15:32:40Z

Rebased and pushed a commit to use the same ticks as the colorbar (improved consistency), and fix the ylim while browsing. If people want it to change to something else, they can use plt commands or manually change with the matplotlib interactive tools.

lgtm-com · 2019-09-13T16:01:49Z

This pull request fixes 1 alert when merging b8c52cb into 9585e93 - view on LGTM.com

fixed alerts:

1 for Wrong name for an argument in a call

larsoner · 2019-09-13T16:27:22Z

@jasmainak can you have another look?

jasmainak · 2019-09-14T05:17:00Z

I'll take a look tomorrow!

jasmainak · 2019-09-15T00:08:33Z

The ylim works beautifully but I have this issue on the mac when I try to zoom in:

The butterfly plot however looks fine when I zoom in.

mne/viz/_3d.py

larsoner · 2019-09-15T01:27:05Z

Removed the problematic clip_on=False and improved doc description

lgtm-com · 2019-09-15T01:55:28Z

This pull request fixes 1 alert when merging 5ce5f19 into d9e2788 - view on LGTM.com

fixed alerts:

1 for Wrong name for an argument in a call

jasmainak · 2019-09-15T02:58:30Z

examples/inverse/plot_lcmv_beamformer_volume.py

+stc.copy().crop(0.05, 0.18).plot(
+    src=morph, subject='fsaverage', subjects_dir=subjects_dir,
+    mode='stat_map', clim=dict(kind='value', pos_lims=lims),
+    initial_time=0.1, verbose=True)


why is this so slow compared to the other two plots? Does the morph do some computation on demand? It's a bit hard to interact with the time series on this one, specially if you click on a time point very close to the vertical line (maybe because the samples are far apart?).

That said, this PR is already a big improvement over what exists. So, feel free to merge it. I won't have time to look again the next 2 days.

agramfort · 2019-09-15T09:35:05Z

thx @larsoner !

* ENH: Add initial arguments to vol viz * DOC: Links * FIX: Fix for deprecation * WIP * FIX: Fix affine treatment and vertices order * FIX: Adjust tests * FIX: Flake * FIX: Check * BUG: Fix axis limits * FIX: Clip and desc

`Forward` keys added by: * 6d84371 / mne-tools#5840 / 2019-01-29 `SourceMorph` keys added by: * 6d84371 / mne-tools#5840 / 2019-01-29 * b65a045 / mne-tools#6046 / 2019-09-15

dictionary key `name` repeated with different values `Forward` keys added by: * 6d84371 / mne-tools#5840 / 2019-01-29 `SourceMorph` keys added by: * 6d84371 / mne-tools#5840 / 2019-01-29 * b65a045 / mne-tools#6046 / 2019-09-15

jasmainak reviewed Mar 12, 2019

View reviewed changes

mne/viz/tests/test_3d.py Show resolved Hide resolved

jasmainak reviewed Mar 14, 2019

View reviewed changes

examples/inverse/plot_lcmv_beamformer_volume.py Outdated Show resolved Hide resolved

jasmainak reviewed Mar 14, 2019

View reviewed changes

examples/inverse/plot_lcmv_beamformer_volume.py Outdated Show resolved Hide resolved

larsoner added the VIZ label Mar 27, 2019

larsoner force-pushed the initial branch from 08429f6 to e7788e5 Compare April 27, 2019 04:51

larsoner force-pushed the initial branch from e7788e5 to 0a44aa8 Compare September 5, 2019 16:28

larsoner force-pushed the initial branch 2 times, most recently from 7f8e9d6 to 998e936 Compare September 5, 2019 20:38

agramfort reviewed Sep 6, 2019

View reviewed changes

examples/inverse/plot_lcmv_beamformer_volume.py Outdated Show resolved Hide resolved

larsoner mentioned this pull request Sep 11, 2019

MAINT: Refactor morph code to be explicit #6752

Closed

larsoner force-pushed the initial branch from 998e936 to bfef83a Compare September 11, 2019 17:42

larsoner changed the title ~~WIP: Add initial arguments to vol viz~~ MRG: Add initial arguments to vol viz Sep 11, 2019

larsoner added this to the 0.19 milestone Sep 11, 2019

agramfort approved these changes Sep 11, 2019

View reviewed changes

larsoner added 9 commits September 13, 2019 10:35

ENH: Add initial arguments to vol viz

b65a045

DOC: Links

98d3fbc

FIX: Fix for deprecation

f8488c6

WIP

9905635

FIX: Fix affine treatment and vertices order

13e593f

FIX: Adjust tests

c8846cd

FIX: Flake

a27f006

FIX: Check

d01a1e7

BUG: Fix axis limits

b8c52cb

larsoner force-pushed the initial branch from 1b5c4bd to b8c52cb Compare September 13, 2019 15:31

jasmainak reviewed Sep 15, 2019

View reviewed changes

mne/viz/_3d.py Outdated Show resolved Hide resolved

FIX: Clip and desc

5ce5f19

jasmainak reviewed Sep 15, 2019

View reviewed changes

jasmainak approved these changes Sep 15, 2019

View reviewed changes

agramfort merged commit 951df43 into mne-tools:master Sep 15, 2019

larsoner deleted the initial branch September 15, 2019 13:19

larsoner mentioned this pull request Sep 19, 2019

Morph volumetric source estimate--discrepancy in number of vertices #6265

Closed

DimitriPapadopoulos mentioned this pull request Mar 22, 2023

DOC, MAINT: Fix pycodestyle issue F601 #11585

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MRG: Add initial arguments to vol viz #6046

MRG: Add initial arguments to vol viz #6046

larsoner commented Mar 11, 2019 •

edited

Loading

codecov bot commented Mar 12, 2019 •

edited

Loading

jasmainak commented Mar 12, 2019

larsoner commented Mar 12, 2019

larsoner commented Mar 17, 2019

jasmainak commented Mar 17, 2019

britta-wstnr commented Apr 11, 2019

larsoner commented Apr 11, 2019

lgtm-com bot commented Apr 27, 2019

lgtm-com bot commented Sep 5, 2019

lgtm-com bot commented Sep 5, 2019

larsoner commented Sep 11, 2019 •

edited

Loading

lgtm-com bot commented Sep 11, 2019

agramfort left a comment

larsoner commented Sep 12, 2019

larsoner commented Sep 13, 2019

lgtm-com bot commented Sep 13, 2019

larsoner commented Sep 13, 2019

jasmainak commented Sep 14, 2019

jasmainak commented Sep 15, 2019

larsoner commented Sep 15, 2019

lgtm-com bot commented Sep 15, 2019

jasmainak Sep 15, 2019

agramfort commented Sep 15, 2019

MRG: Add initial arguments to vol viz #6046

MRG: Add initial arguments to vol viz #6046

Conversation

larsoner commented Mar 11, 2019 • edited Loading

codecov bot commented Mar 12, 2019 • edited Loading

Codecov Report

jasmainak commented Mar 12, 2019

larsoner commented Mar 12, 2019

larsoner commented Mar 17, 2019

jasmainak commented Mar 17, 2019

britta-wstnr commented Apr 11, 2019

larsoner commented Apr 11, 2019

lgtm-com bot commented Apr 27, 2019

lgtm-com bot commented Sep 5, 2019

lgtm-com bot commented Sep 5, 2019

larsoner commented Sep 11, 2019 • edited Loading

lgtm-com bot commented Sep 11, 2019

agramfort left a comment

Choose a reason for hiding this comment

larsoner commented Sep 12, 2019

larsoner commented Sep 13, 2019

lgtm-com bot commented Sep 13, 2019

larsoner commented Sep 13, 2019

jasmainak commented Sep 14, 2019

jasmainak commented Sep 15, 2019

larsoner commented Sep 15, 2019

lgtm-com bot commented Sep 15, 2019

jasmainak Sep 15, 2019

Choose a reason for hiding this comment

agramfort commented Sep 15, 2019

larsoner commented Mar 11, 2019 •

edited

Loading

codecov bot commented Mar 12, 2019 •

edited

Loading

larsoner commented Sep 11, 2019 •

edited

Loading