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

mne make_scalp_surfaces does not recompute mkheadsurf #13014

Open
vferat opened this issue Dec 9, 2024 · 3 comments · May be fixed by #13024
Open

mne make_scalp_surfaces does not recompute mkheadsurf #13014

vferat opened this issue Dec 9, 2024 · 3 comments · May be fixed by #13024
Labels
Milestone

Comments

@vferat
Copy link
Contributor

vferat commented Dec 9, 2024

Description of the problem

When running mne make_scalp_surfaces with a new MRI specified using the -m argument, the command does not rerun mkheadsurf if it has already been run previously. Consequently, the changes to the MRI are not accounted for.

This behavior appears to be related to the check_seghead function (source code link). This function returns the previously computed lh.seghead or lh.smseghead file, which prevents mne make_scalp_surfaces from rerunning mkheadsurf.

The issue might also affect the threshold parameter.

Workaround:

The current workaround is to manually delete the lh.segheador lh.smseghead file.

Suggested Fix:

Force rerunning mkheadsurf when the overwrite parameter is set to True.

Steps to reproduce

Run
`mne make_scalp_surfaces --subject sample`

then run the same command specifying another MRI (or threshold):

`mne make_scalp_surfaces --subject sample -m orig.mgz`

Link to data

Context

I'm trying to compute scalp surfaces from a MP2RAGE scan. Because of the presence of noise outside the head, I need to provide another (manually preprocessed) MRI as an input to mne make_scalp_surfaces otherwise the head surface can't be used for coregistration.

Expected results

Recompute results using the new MRI/threshold

Actual results

Use previously computed results.

Additional information

Platform Linux-5.15.167.4-microsoft-standard-WSL2-x86_64-with-glibc2.35
Python 3.11.9 | packaged by conda-forge | (main, Apr 19 2024, 18:36:13) [GCC 12.3.0]
Executable /opt/conda/envs/smriprep/bin/python3.11
CPU x86_64 (32 cores)
Memory 62.6 GB

Core
├☑ mne 1.8.0 (latest release)
├☑ numpy 1.26.4 (OpenBLAS 0.3.28 with 1 thread)
├☑ scipy 1.13.1
└☑ matplotlib 3.9.1 (backend=Agg)

Numerical (optional)
├☑ sklearn 1.4.2
├☑ nibabel 5.2.1
├☑ nilearn 0.10.4
├☑ pandas 2.2.2
├☑ h5io 0.2.4
├☑ h5py 3.11.0
└☐ unavailable numba, dipy, openmeeg, cupy

Visualization (optional)
├☑ pyvista 0.44.2 (OpenGL unavailable)
├☑ pyvistaqt 0.11.1
├☑ vtk 9.3.1
├☑ qtpy 2.4.2 (None=None)
└☐ unavailable ipympl, pyqtgraph, mne-qt-browser, ipywidgets, trame_client, trame_server, trame_vtk, trame_vuetify

Ecosystem (optional)
├☑ mne-bids 0.16.0
└☐ unavailable mne-nirs, mne-features, mne-connectivity, mne-icalabel, mne-bids-pipeline, neo, eeglabio, edfio, mffpy, pybv

@vferat vferat added the BUG label Dec 9, 2024
@larsoner
Copy link
Member

larsoner commented Dec 9, 2024

Does it do it if you pass the -o argument?

  -o, --overwrite       Overwrite previously computed surface

If so maybe we "just" need to document this better?

Though really I'm not sure when you wouldn't want to overwrite the previous one, so we could consider a bugfix and just always essentially set -o...

@vferat
Copy link
Contributor Author

vferat commented Dec 9, 2024

The --overwrite argument does not appear to have any effect on the current issue.

The current workflow seems to generate seghead.mgz -> lh.seghead -> XX-head-dense.fif .

Currently the overwrite parameter only prevents overwritting the XX-head-dense.fif file.
The seghead.mgz and lh.seghead files are never recomputed after the first call.

The overwrite is useful to avoid changing files. The only other reason you would like to rerun the command without overwriting things is to compute the “medium” and “sparse” decimations from the current head segmentation in case one previously ran the command with the no_decimate option. This is an extremely rare case, and recomputing everything is relatively quick.

I would keep the overwrite parameter, but change the current behavior:

  • if either seghead.mgz , lh.seghead or XX-head-dense.fif exists and overwrite is not set -> raise error
  • if overwrite is set -> force rerunning everything including seghead.mgz and lh.seghead

@larsoner
Copy link
Member

larsoner commented Dec 9, 2024

Seems reasonable to me!

@vferat vferat linked a pull request Dec 12, 2024 that will close this issue
@larsoner larsoner added this to the 1.10 milestone Dec 16, 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 a pull request may close this issue.

2 participants