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

Stack vertices in plot_volume_source_estimates #12025

Merged
merged 10 commits into from
Oct 2, 2023

Conversation

mscheltienne
Copy link
Member

@mscheltienne mscheltienne commented Sep 28, 2023

MWE failing on main
from mne import (
    compute_covariance,
    make_fixed_length_epochs,
    make_forward_solution,
    read_bem_solution,
    setup_volume_source_space,
)
from mne.datasets import sample
from mne.io import read_raw_fif
from mne.minimum_norm import apply_inverse, make_inverse_operator


directory = sample.data_path() / "MEG" / "sample"
raw = read_raw_fif(directory / "sample_audvis_raw.fif", preload=False)
raw.pick("meg").crop(0, 60).load_data()
epochs = make_fixed_length_epochs(raw, preload=True)
evoked = epochs.average()

subjects_dir = sample.data_path() / "subjects"
subject = "sample"

bem = read_bem_solution(subjects_dir / f"{subject}" / "bem" / "sample-5120-bem-sol.fif")

pos = 10.0  # spacing in mm
volume_label = [
    "Right-Cerebral-Cortex",
    "Left-Cerebral-Cortex",
]
src = setup_volume_source_space(
    subject,
    subjects_dir=subjects_dir,
    pos=pos,
    mri=subjects_dir / subject / "mri" / "aseg.mgz",
    bem=bem,
    volume_label=volume_label,
    add_interpolator=True,
)

trans = sample.data_path() / "MEG" / "sample" / "sample_audvis_raw-trans.fif"
fwd = make_forward_solution(
    evoked.info,
    trans,
    src,
    bem,
    meg=True,
    eeg=False,
    mindist=5.0,  # ignore sources <= 5mm from the inner skull
    n_jobs=6,
)

cov = compute_covariance(
    epochs,
    tmin=None,
    tmax=None,
    method="empirical",
)
inverse_operator = make_inverse_operator(evoked.info, fwd, cov, loose=1, depth=0.8)

lambda2 = 1.0 / 3**2
stc = apply_inverse(evoked, inverse_operator, lambda2, method="sLORETA", pick_ori=None)

stc.plot(src, subject, subjects_dir, initial_time=0.03)

This PR creates the variables vertices = np.hstack(stc.vertices) instead of relying on stc.vertices[0]. On main, VolSourceEstimate.plot can not plot on the left side of the brain (at least with a source space created as above) since the vertices of the left side are stored in stc.vertices[1]. With the MWE example above, it raises:

  Cell In[1], line 62
    stc.plot(src, subject, subjects_dir, initial_time=0.03)

  File ~/git/mscheltienne/mne-python/mne/source_estimate.py:2264 in plot
    return plot_volume_source_estimates(

  File <decorator-gen-144>:12 in plot_volume_source_estimates

  File ~/git/mscheltienne/mne-python/mne/viz/_3d.py:2925 in plot_volume_source_estimates
    ijk = stc_ijk[loc_idx]

IndexError: index 305 is out of bounds for axis 0 with size 173

With this PR, I get the plot:

f

Does this change makes sense to everyone, and is it the correct way to fix this indexing error (i.e. is this fix taking the correct slice)?

@larsoner
Copy link
Member

Looks correct to me. don't forget to change devel.rst as a bugfix. Would it be possible to add a test?

@mscheltienne
Copy link
Member Author

Glad it looks OK. I'll prune the MWE I got to make a test out of it.

@mscheltienne
Copy link
Member Author

I did not find a src, fwd, inv to load from the testing dataset, thus I used a quicker version of the MWE above.
On an intel-based macOS: the test takes 793 ms ± 4.1 ms per loop.

@larsoner larsoner enabled auto-merge (squash) October 2, 2023 13:47
@larsoner
Copy link
Member

larsoner commented Oct 2, 2023

Marking for merge when green, thanks in advance @mscheltienne !

@larsoner larsoner merged commit 578f2a9 into mne-tools:main Oct 2, 2023
28 checks passed
@mscheltienne mscheltienne deleted the dev2 branch October 2, 2023 17:47
@larsoner larsoner added the BUG label Oct 2, 2023
snwnde pushed a commit to snwnde/mne-python that referenced this pull request Mar 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants