Fixes make_scalp_surfaces

[vferat]

Reference issue (if any)

Fixes #13014.

What does this implement/fix?

Updates make_scalp_surfaces to force regenerating seghead.mgz ,lh.seghead and {subject}-head-{{}}.fif if overwrite is set to True. Will raise an OSError if either seghead.mgz ,lh.seghead and {subject}-head-{{}}.fif exists and overwrite is set to False

Additional information

There are some edgy cases when mixing overwrite and no_decimate which can lead to {subject}-head-medium.fif / {subject}-head-sparse.fif to be untouched while seghead.mgz ,lh.seghead and {subject}-head-dense.fif are regenerated.

I tried to handle theses cases by always deleting {subject}-head-medium.fif / {subject}-head-sparse.fif if overwrite=True (even if no_decimate=True) and by raising and specific error if overwrite=False , no_decimate=True but {subject}-head-medium.fif / {subject}-head-sparse.fif exist.

[larsoner]

@vferat any way I can help with this one?

[vferat]

Hey @larsoner,

Sorry for the lack of responsiveness, projects are coming thick and fast and it's becoming difficult to find time to contribute.

The remaining error is a difference between the default sample-head-dense.fif shipped with sample data and the newly computed sample-head-dense.fif.

• /tmp/pytest-of-ferat/pytest-8/test_make_scalp_surfaces0/sample/bem/sample-head-dense.fif
• /home/ferat/mne_data/MNE-testing-data/subjects/sample/bem/sample-head-dense.fif

I noticed that:

• seghead.mgz from the sample data is 10 kb
• seghead.mgz from running mne make_scalp_surfaces on main is still 10kb (computation skipped)
• seghead.mgz from running mne make_scalp_surfaces on this PR is 9kb (recomputed)

On main, tests skip the computation of seghead.mgz (unwanted behavior) therefore sample-head-dense.fif is unchanged compared to the sample dataset -> test pass

However, on this PR tests recompute seghead.mgz therefore sample-head-dense.fif is changed compared to the sample dataset -> test fails

It seems that the seghead.mgz file from sample data is not the output of mkheadsurf -subjid sample -srcvol T1.mgz -thresh1 20 -thresh2 20. Could you confirm that ?

EDIT: I can confirm that the output of mkheadsurf -subjid sample -srcvol T1.mgz -thresh1 20 -thresh2 20 is different from the shipped seghead.mgz , at least on my local setup (freesurfer 7.4.1).

Would it be ok to remove the validation step against the shipped seghead.mgz or update the seghead.mgz in MNE sample dataset ?

[larsoner]

It seems that the seghead.mgz file from sample data is not the output of mkheadsurf -subjid sample -srcvol T1.mgz -thresh1 20 -thresh2 20. Could you confirm that ?

I hoped I would find it in https://github.com/search?q=repo%3Amne-tools%2Fmne-scripts+seghead&type=code but I don't. Similar calls there are just mkheadsurf -s ${SUBJECT}... but it could be on some (very!) old version of FreeSurfer so who knows.

On main, tests skip the computation of seghead.mgz (unwanted behavior) therefore sample-head-dense.fif is unchanged compared to the sample dataset -> test pass

Should we add a recompute_seghead=True (new default) that you could set to False for the purposes of the test? It would also go faster presumably...

Would it be ok to remove the validation step against the shipped seghead.mgz or update the seghead.mgz in MNE sample dataset ?

Updating the dataset doesn't seem worth it (better to preserve it as-is), I think it's okay to adjust the test. It would be great if we could assert the meshes are similar, but it doesn't seem trivial on a quick search (even with VTK) so I think it's okay to skip that part

[vferat]

Should we add a recompute_seghead=True (new default) that you could set to False for the purposes of the test? It would also go faster presumably...

This may cause confusion with other parameters such as overwrite, but it may not be such a problem for a optional parameter.. let me know what you think is the best.

Updating the dataset doesn't seem worth it (better to preserve it as-is), I think it's okay to adjust the test. It would be great if we could assert the meshes are similar, but it doesn't seem trivial on a quick search (even with VTK) so I think it's okay to skip that part

I'v removed the test for now and I'll add it back it if we choose to add the recompute_seghead parameter.

[larsoner]

This may cause confusion with other parameters such as overwrite, but it may not be such a problem for a optional parameter.. let me know what you think is the best.

I think as long as we say what overwrite controls the overwriting of -- which I'm not sure of offhand -- I think we're okay. Do you see this as a parameter that would be useful to end users? I'm guessing yes, since overwrite is currently used, there are probably some people who don't want seghead recomputed, but do want the other stuff after that recomputed...

[vferat]

I've added a reuse_seghead parameter to mne.bem.make_scalp_surfaces instead of recompute_seghead (opposite behavior) to keep the same API for the mne_make_scalp_surfaces command (--reuse-seghead flag).

I've also added back the surface checks when reuse_seghead=True and some other test cases for combinations of overwrite and reuse_seghead values.

[larsoner]

In it goes, thanks @vferat !