Skip to content

Add sourcespace to Report #12848

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

Open
wants to merge 32 commits into
base: main
Choose a base branch
from
Open

Add sourcespace to Report #12848

wants to merge 32 commits into from

Conversation

vferat
Copy link
Contributor

@vferat vferat commented Sep 13, 2024

Reference issue (if any)

Fixes #12836

What does this implement/fix?

Add a src argument to the Report.add_bem method.

Additional information

I'm not sure which strategy to adopt and changes to make to Report.parse_folder.
The Report.parse_folder method automatically adds BEM to the Report if Report.subject is specified. However, several sourcespaces -src.fif files might be present in the BEM folder and/or folder to parse.

@larsoner
Copy link
Member

For add_folder I think if a source space is found, it could be passed in during the add_bem step (assuming that happens in parse_folder, I don't remember!). Not 100% sure the right thing to do when multiple source spaces are found, could just put in whichever is first, or only add one if there is exactly one source space. I'm thinking just using the first one would make sense so that you get some sense whether or not the alignment is correct.

Another place it would be nice to add if you're motivated is in report.add_trans since that's used for making sure everything is nicely aligned, too.

Then it would be good to pass some src in the tests for report.add_bem and report.add_trans

@vferat
Copy link
Contributor Author

vferat commented Sep 16, 2024

Having looked at the many possibilities, I think the best solution would be to add a figure in the forward section:
the forward object contains all the information needed to display the source space

This solution has the advantage of being able to also display the EEG sensors projected onto the scalp, as well as displaying only the sources used in for forward computation (as some sources from the original source space may be dropped if too close to the inner skull surface). This will give a global idea of the sensors <-> sources model.

mne.viz.plot_alignment(trans=fwd["mri_head_t"], info=fwd["info"], src=fwd["src"], eeg=dict(original=0.2, projected=0.8))

image

We could also have a clearer view of the source space with an additional plot:

image

@larsoner
Copy link
Member

I like both of those plots! Adding to the forward section sounds reasonable to me

@vferat
Copy link
Contributor Author

vferat commented Feb 24, 2025

  • If subject is defined, the forward report section now includes new figures:
    • Alignement with projected EEG sensors, MEG sensors, source spaces, head model
    • BEM with sources
    • sources (3D view)
report = mne.Report(verbose=True, subject='sample', subjects_dir=subjects_dir)
report.add_forward(fwd, title="Forward")
image
  • If subject is not defined, keeps the default forward report section:
report = mne.Report(verbose=True)
report.add_forward(fwd, title="Forward")
image

@vferat vferat marked this pull request as ready for review February 24, 2025 10:17
@larsoner
Copy link
Member

pip-pre failures are unrelated and are being tackled in #13125, I should be able to review soon!

@hoechenberger
Copy link
Member

Hi, what's with the Source space row here? Seems there's something wrong with the text replacement:

image

@vferat
Copy link
Contributor Author

vferat commented Mar 17, 2025

image

1 - Alignment View

It seems that the Head & sensor digitization points are not saved in the forward["info"] structure:

data_dir = testing.data_path(download=True)
subjects_dir = data_dir / "subjects"
sample_meg_dir = data_dir / "MEG" / "sample"
fwd_fname = sample_meg_dir / "sample_audvis_trunc-meg-eeg-oct-6-fwd.fif"
fwd = mne.read_forward_solution(fwd_fname)
fwd["info"]['dig']

Output:

KeyError: 'dig'

Since this data is missing, I removed the caption from the alignment image.


2 - Source Space(s) View

The view isn't fitting well because the view settings are manually defined here for rendering with a head surface.

I see two possible fixes:

  • Add the head surface (see the image in this post).
  • Create new view definitions for plotting without a head surface.

However, I'm not 100% sure this view is really needed.

Additionally, I'm not sure why there are two front views but no back view in the current view definitions.

@larsoner
Copy link
Member

Additionally, I'm not sure why there are two front views but no back view

Good question, a back one would be good. It would also be good to have a frontal that was a little bit from the opposite side as the first one (like the first image in the first row has the head turned a little to the right, would be nice if that row ended with a symmetric one with the head turned a little to the left).

If we add that to the first row to get a to a 4 col 2 row layout, then replace the second "straight on front" (currently second row third image) view with one from the back that would give us 7 images (4 first row, 3 second row) which could be good.

@vferat
Copy link
Contributor Author

vferat commented Apr 7, 2025

Additionally, I'm not sure why there are two front views but no back view

Good question, a back one would be good. It would also be good to have a frontal that was a little bit from the opposite side as the first one (like the first image in the first row has the head turned a little to the right, would be nice if that row ended with a symmetric one with the head turned a little to the left).

If we add that to the first row to get a to a 4 col 2 row layout, then replace the second "straight on front" (currently second row third image) view with one from the back that would give us 7 images (4 first row, 3 second row) which could be good.

image

What about this 5x2 layout ?

@larsoner
Copy link
Member

larsoner commented Apr 7, 2025

I like it!

@vferat vferat requested a review from wmvanvliet as a code owner April 14, 2025 11:29
@larsoner
Copy link
Member

Let me know when I should review!

@larsoner larsoner added this to the 1.10 milestone Apr 18, 2025
@vferat
Copy link
Contributor Author

vferat commented May 26, 2025

I've finally found some time to sort out the last issues.
Ready for review @larsoner

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.

MNE Report: plot source space(s)
3 participants