Skip to content

Conversation

@effigies
Copy link
Member

@effigies effigies commented Sep 26, 2025

sdcflows adopted fit-transform before other nipreps. One thing we've found works well to ensure the same behavior with precomputed derivatives is to save the derivative and pass the saved derivative to the downstream workflow.

This does this for preprocessed fieldmaps, references, coefficients, and (newly) masks. Masks are used by downstream tools to register the fieldmap to the target image. Although they are not needed for application, if the registration is available, they do allow fieldmaps to be precomputed and then used in a new run.

There are a couple cleanup commits in here. The two that are meaningful are labeled feat:.

Enabling nipreps/fmriprep#3532.

@codecov
Copy link

codecov bot commented Sep 26, 2025

Codecov Report

❌ Patch coverage is 90.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 84.13%. Comparing base (087b172) to head (38e9b63).
⚠️ Report is 9 commits behind head on main.

Files with missing lines Patch % Lines
sdcflows/workflows/outputs.py 90.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #506      +/-   ##
==========================================
- Coverage   84.15%   84.13%   -0.02%     
==========================================
  Files          30       30              
  Lines        2872     2875       +3     
  Branches      382      380       -2     
==========================================
+ Hits         2417     2419       +2     
  Misses        381      381              
- Partials       74       75       +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.

effigies added a commit to nipreps/fmriprep that referenced this pull request Sep 26, 2025
Copy link
Contributor

@mgxd mgxd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In favor of this - let's quickly release a niworkflow patch and remove the patterns hack

Comment on lines +233 to +239
ds_mask._interface._file_patterns += (
'sub-{subject}[/ses-{session}]/{datatype<fmap>|fmap}/'
'sub-{subject}[_ses-{session}][_hash-{hash}][_acq-{acquisition}]'
'[_dir-{direction}][_run-{run}][_part-{part}][_space-{space}]'
'[_cohort-{cohort}][_res-{resolution}][_fmapid-{fmapid}]'
'[_desc-{desc}]_{suffix<mask>}{extension<.nii|.nii.gz|.json>|.nii.gz}',
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

opened nipreps/niworkflows#977 to make this less hacky

@effigies
Copy link
Member Author

@mgxd Pushed a niworkflows tag, and will update this shortly. I don't plan to wait on another CI round, and will depend on the CI for the release to catch if I mess something up.

Should this be 2.14.1 or 2.15.0? It makes very little difference.

@mgxd
Copy link
Contributor

mgxd commented Sep 26, 2025

2.15.0 - this will invalidate a significant amount of downstream caches

effigies added a commit that referenced this pull request Sep 26, 2025
feat: Save mask to derivatives, pass derivatives to outputnode
@effigies effigies merged commit 38e9b63 into nipreps:main Sep 26, 2025
17 checks passed
@effigies effigies deleted the feat/derivatives branch September 26, 2025 14:38
effigies added a commit that referenced this pull request Sep 26, 2025
2.15.0 (September 26, 2025)

Feature release in the 2.15.x series.

This release reworks derivatives a bit to ensure that everything needed for
downstream processing is placed in the output directory.
The results from the output directory are aggregated into the workflow outputnode,
allowing consistency in workflows that can accept precomputed derivatives.

* feat: Save mask to derivatives, pass derivatives to outputnode (#506)
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.

2 participants