-
Notifications
You must be signed in to change notification settings - Fork 296
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
FIX: Write out boldref-space brain mask with minimal level #3292
Conversation
Traveling so can't look closely, but does this avoid double-sinking in cases where boldref outputs are requested? |
Ah good point. It doesn't yet, so I'll switch this to a draft until I've got everything working and there's no double-sinking. |
fmriprep/workflows/bold/outputs.py
Outdated
ds_boldmask = pe.Node( | ||
DerivativesDataSink( | ||
base_directory=output_dir, | ||
datatype='func', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if this is a good idea, but for some reason the files were written out to anat
otherwise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this will be fixed by passing in an original bold file.
fmriprep/workflows/bold/fit.py
Outdated
|
||
# fmt:off | ||
workflow.connect([ | ||
(hmcref_buffer, fmapref_buffer, [("boldref", "boldref_files")]), | ||
(fmapref_buffer, enhance_boldref_wf, [("out", "inputnode.in_file")]), | ||
(fmapref_buffer, ds_coreg_boldref_wf, [("out", "inputnode.source_files")]), | ||
(ds_coreg_boldref_wf, regref_buffer, [("outputnode.boldref", "boldref")]), | ||
(fmapref_buffer, ds_boldmask_wf, [("out", "inputnode.source_files")]), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other workflows just set
ds_hmc_wf.inputs.inputnode.source_files = [bold_file]
I don't think I would worry about getting sbref vs bold straight here, as it's just a mask.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems fine. It does raise the question of what to do if a pre-computed mask or coregistration transform is found, but that can be handled in a separate issue. This one is sufficient for a bug-fix.
23.2.3 (May 20, 2024) Bug fix release in the 23.2.x series. Writes brain masks in ``space-boldref`` with ``--level minimal``, bringing behavior in line with documentation. * FIX: Write out boldref-space brain mask with minimal level (#3292)
Closes #3290.
Changes proposed in this pull request
init_ds_boldmask_wf
workflow. This might be overkill.init_bold_fit_wf
.init_ds_bold_native_wf
, since it's written out ininit_bold_fit_wf
now.