Skip to content

Conversation

@mgxd
Copy link
Contributor

@mgxd mgxd commented Mar 6, 2025

This PR makes the use of a prior during SyN SDC optional, since distortions may differ depending on population. Eventually, it would be nice to add a range of priors, either within SDCFlows directly or easily plugged in from other tools.

Relevant to nipreps/nibabies#393

mgxd added 2 commits March 6, 2025 15:29
Since these each workflow is called by the `FieldmapEstimation` class,
these workflows need to be robust enough to handle keyword arguments
designed for one of the specific workflows (in this case SyN)
@codecov
Copy link

codecov bot commented Mar 6, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.87%. Comparing base (5a55a81) to head (d2de7ba).
Report is 7 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #480      +/-   ##
==========================================
+ Coverage   83.85%   83.87%   +0.01%     
==========================================
  Files          30       30              
  Lines        2819     2822       +3     
  Branches      365      366       +1     
==========================================
+ Hits         2364     2367       +3     
  Misses        384      384              
  Partials       71       71              

☔ 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.

@mgxd mgxd marked this pull request as ready for review March 6, 2025 22:06
@mgxd mgxd requested review from effigies and oesteban March 7, 2025 15:07
@effigies
Copy link
Member

effigies commented Mar 7, 2025

Could you show a command.txt from a syn_sdc_wf.syn node?

@mgxd
Copy link
Contributor Author

mgxd commented Mar 7, 2025

Sure, here's from a current dev run in nibabies

antsRegistration --collapse-output-transforms 1 \
--dimensionality 3 --initialize-transforms-per-stage 0 \
--interpolation Linear --output fmap_syn \
--transform SyN[ 0.8, 6.0, 3.0 ] \
--metric Mattes[ /scratch/nibabies_25_0_wf/single_subject_sub-X_ses-V02_wf/syn_preprocessing_auto_00001/anat2epi/clipped_noise_corrected_trans.nii.gz, /scratch/nibabies_25_0_wf/single_subject_sub-X_ses-V02_wf/fmap_preproc_wf/wf_auto_00001/clip_epi/clipped.nii.gz, 0.5, 48, Random, 0.8 ] \
--metric Mattes[ /scratch/nibabies_25_0_wf/single_subject_sub-X_ses-V02_wf/fmap_preproc_wf/wf_auto_00001/lap_anat_norm/clipped_noise_corrected_trans_maths_norm.nii.gz, /scratch/nibabies_25_0_wf/single_subject_sub-X_ses-V02_wf/fmap_preproc_wf/wf_auto_00001/lap_epi_norm/clipped_maths_norm.nii.gz, 0.5, 48, Random, 0.8 ] \
--convergence [ 200x100, 1e-06, 5 ] --smoothing-sigmas 2.0x0.0vox --shrink-factors 1x1 \
--use-histogram-matching 1 --restrict-deformation 0.1x1.0x0.1 \
--masks [ /scratch/nibabies_25_0_wf/single_subject_sub-X_ses-V02_wf/syn_preprocessing_auto_00001/mask2epi/sub-X_ses-V02_space-T2w_desc-brain_mask_trans_uint8.nii.gz, /scratch/nibabies_25_0_wf/single_subject_X_ses-V02_wf/fmap_preproc_wf/wf_auto_00001/epi_umask/clipped_mask_union.nii.gz ] \
--transform SyN[ 0.8, 2.0, 1.0 ] \
--metric Mattes[ /scratch/nibabies_25_0_wf/single_subject_sub-X_ses-V02_wf/syn_preprocessing_auto_00001/anat2epi/clipped_noise_corrected_trans.nii.gz, /scratch/nibabies_25_0_wf/single_subject_sub-X_ses-V02_wf/fmap_preproc_wf/wf_auto_00001/clip_epi/clipped.nii.gz, 0.5, 48 ] \
--metric Mattes[ /scratch/nibabies_25_0_wf/single_subject_sub-X_ses-V02_wf/fmap_preproc_wf/wf_auto_00001/lap_anat_norm/clipped_noise_corrected_trans_maths_norm.nii.gz, /scratch/nibabies_25_0_wf/single_subject_sub-X_ses-V02_wf/fmap_preproc_wf/wf_auto_00001/lap_epi_norm/clipped_maths_norm.nii.gz, 0.5, 48 ] \
--convergence [ 10, 1e-08, 2 ] \
--smoothing-sigmas 0.0vox --shrink-factors 1 --use-histogram-matching 1 --restrict-deformation 0.1x1.0x0.1 \
--masks [ /scratch/nibabies_25_0_wf/single_subject_sub-X_ses-V02_wf/syn_preprocessing_auto_00001/mask2epi/sub-X_ses-V02_space-T2w_desc-brain_mask_trans_uint8.nii.gz, /scratch/nibabies_25_0_wf/single_subject_sub-X_ses-V02_wf/fmap_preproc_wf/wf_auto_00001/epi_umask/clipped_mask_union.nii.gz ] \
--winsorize-image-intensities [ 0.001, 0.998 ]  --write-composite-transform 0

@effigies
Copy link
Member

effigies commented Mar 7, 2025

Okay. Cross-referencing #291, it looks like the third mask is never used in any case because we only have two registration stages.

From antsRegistraion --help:

 -x, --masks [fixedImageMask,movingImageMask]
      Image masks to limit voxels considered by the metric. Two options are allowed 
      for mask specification: 1) Either the user specifies a single mask to be used 
      for all stages or 2) the user specifies a mask for each stage. With the latter 
      one can select to which stages masks are applied by supplying valid file names. 
      If the file does not exist, a mask will not be used for that stage. Note that we 
      handle the fixed and moving masks separately to enforce this constraint. 

Given that we're not using the prior at all, does it make sense to remove it altogether and then we can consider re-adding it in a concerted PR?

I think we can leave the inputspec the same, for backwards compatibility, just don't ever hook up sd_prior.

@mgxd
Copy link
Contributor Author

mgxd commented Mar 7, 2025

@effigies is d2de7ba what you meant?

@mgxd mgxd force-pushed the enh/syn-no-prior branch from 51aae84 to d2de7ba Compare March 7, 2025 18:09
Copy link
Member

@effigies effigies left a comment

Choose a reason for hiding this comment

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

Yeah, I think this looks good. syn-sdc is a mess and I really am not looking forward to figuring out how to get the prior working again.

@effigies effigies merged commit 4b1f566 into nipreps:master Mar 7, 2025
18 checks passed
@mgxd mgxd deleted the enh/syn-no-prior branch March 7, 2025 19:19
effigies added a commit that referenced this pull request Mar 21, 2025
2.12.0 (March 21, 2025)

Feature release in the 2.12.x series.

This release migrates from the deprecated ``niworkflows.reporting``
module to the ``nireports`` package.

* FIX: AttributeError for _ApplyCoeffsFieldInputSpec (#481)
* ENH: Allow running SyN SDC without using prior (#480)
* ENH: Allow estimated and fallback TotalReadoutTime (#477)
* RF: Transition from niworkflows reporting interfaces (#473)
* DOC: Fix broken link [skip ci] (#482)
* MNT: Add `defaults` to `conda` channels in `build-test-publish` GHA (#474)
* MNT: Update `niworkflows` version to 1.11.0 (#478)
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