Allow configuration of --no-csf in SynthStrip

[psadil]

Changes proposed in this pull request

Allow configuration of the --no-csf option in the SynthStrip workflow and by default set it to True (currently implies False). Keep the currently implied default (False).

Closes #954

Documentation that should be reviewed

---

Note

Adds a --no-csf CLI flag and workflow.no_csf config, plumbing the option into SynthStrip across anatomical, DWI, and fieldmap workflows.

• CLI:
  • Import BooleanOptionalAction and add --no-csf flag in qsiprep/cli/parser.py to control SynthStrip CSF removal.
• Config:
  • Introduce workflow.no_csf boolean in qsiprep/config.py.
• Workflows:
  • Update init_synthstrip_wf to accept no_csf and pass it to FixHeaderSynthStrip/MockSynthStrip.
  • Propagate no_csf=config.workflow.no_csf in:
    • Anatomical volume workflows (init_anat_preproc_wf, T2w preprocessing).
    • DWI reference workflow (init_dwi_reference_wf).
    • PEPOLAR fieldmap workflow (init_extended_pepolar_report_wf).

^{Written by Cursor Bugbot for commit 93e6630. This will update automatically on new commits. Configure here.}

[cursor]

This PR is being reviewed by Cursor Bugbot

Details

Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.

To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

[psadil]

Let me know if this should be configurable from the CLI

[codecov-commenter]

Codecov Report

❌ Patch coverage is 78.57143% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 46.18%. Comparing base (69be226) to head (d0db378).
⚠️ Report is 12 commits behind head on master.

Files with missing lines	Patch %	Lines
qsiprep/workflows/anatomical/volume.py	75.00%	1 Missing and 1 partial ⚠️
qsiprep/workflows/fieldmap/pepolar.py	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1000      +/-   ##
==========================================
+ Coverage   46.15%   46.18%   +0.02%     
==========================================
  Files          65       65              
  Lines        9793     9802       +9     
  Branches     1083     1084       +1     
==========================================
+ Hits         4520     4527       +7     
- Misses       5045     5046       +1     
- Partials      228      229       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[tsalo]

My intuition is that excluding CSF should be a straightforward improvement across the board, so it shouldn't need to be configurable, but I'll defer to @mattcieslak.

[mattcieslak]

I think we should keep it as an option because I'm not sure how it will interact with MSMT. If CSF is one of the tissues it needs to estimate, maybe it's useful to have some extra CSF in the brain mask? We should ask @36000 or @arokem if this is important. Many other models break down in the csf and we end up with implausible values, so keeping CSF out of the mask would be great

Accidental approval

[tsalo]

If we need CSF in the brain mask for reconstruction, but it's causing problems for registration, then maybe we could produce two masks? fMRIPrep produces two boldrefs- one for motion correction and one for coregistration- so there's a precedent for that.

[mattcieslak]

I think this should be a CLI parameter

[tsalo]

@psadil I think we can try out creating two masks in a follow-up PR before the next release, if experts (@36000, @arokem) think CSF is useful for other steps and/or reconstruction, so just making no-csf configurable in the CLI should be sufficient for this PR.

[36000]

To fit MSMT, you do need to estimate response functions, which is usually done by looking at ROIs in the "pure" WM, GM, CSF. So this would break, but I think we can work around that by making our own brain mask internally that includes the CSF for just these calculations. I think pyAFQ would be fine with having no CSF in the brain mask. By default, we do not use MSMT. I can't speak for how other software does this though.

However, I still have two questions: (1) how reliable is this method? I would guess that removing the CSF is much trickier than doing a generic brain mask and (2) would it not be easier for readability and future software to have separate brain masks and CSF masks? Then QSIprep could pass the brain+CSF mask to certain pipelines and only the brain mask to other pipelines.

[psadil]

However, I still have two questions: (1) how reliable is this method? I would guess that removing the CSF is much trickier than doing a generic brain mask and (2) would it not be easier for readability and future software to have separate brain masks and CSF masks? Then QSIprep could pass the brain+CSF mask to certain pipelines and only the brain mask to other pipelines.

1. I haven't see direct measurements of SynthStrip as a way to generate brain masks specifically (that is, with --no-csf), only those that apply to it's default behavior as a skull-stripping tool.

2. Happy to implement whatever's wanted.

[araikes]

MRtrix workflows that do MSMT might break if you do not have a CSF-inclusive mask.

[tsalo]

It sounds like having two brain masks- one for coregistration and one for steps like MSMT- might be the way to go. My initial idea that it would be fine to address that in a separate PR might have been too hasty.

[psadil]

Are there preferences or opinions about how to name the masks? Currently, there is the single *desc-brain_mask.nii.gz. I assume that should be kept (and should refer to the same kind of mask currently produced, w/ CSF). For the other, how about something like *desc-brainnocsf_mask.nii.gz ? Not sure if it would be better to specify how this one is used.

[psadil]

@tsalo , @mattcieslak , sorry, but I'm hitting a small snag. I only just realized that the version of mri_synthstrip bundled into the qsiprep_build image doesn't have the --no-csf option. It was introduced in mri_synthstrip 1.2. What would be the preferred way to handle that? Options I see include (ordered roughly from smallest to largest change)

• bundle the nocsf model
  • the --no-csf option just selects this model
• have qsiprep_build pull mri_synthstrip from the freesurfer/synthstrip image, instead of the freesurfer/freesurfer one
• update the FreeSurfer version in qsiprep_build
  • e.g., v7.3.2 would do it

[psadil]

@tsalo , @mattcieslak , sorry, but I'm hitting a small snag. I only just realized that the version of mri_synthstrip bundled into the qsiprep_build image doesn't have the --no-csf option. It was introduced in mri_synthstrip 1.2. What would be the preferred way to handle that? Options I see include (ordered roughly from smallest to largest change)

• bundle the nocsf model

  • the --no-csf option just selects this model

• have qsiprep_build pull mri_synthstrip from the freesurfer/synthstrip image, instead of the freesurfer/freesurfer one

• update the FreeSurfer version in qsiprep_build

  • e.g., v7.3.2 would do it

The latest qsiprep_build solves this issue (now at 7.4.1), so I think this is ready for review

[psadil]

Sorry, just following up on this. Any update here, or something I can do to help move this forward?