REGISTRATION overhaul !

[AlexVCaron]

Type of improvement

If submitting a new module or fixing a bug, please use the appropriate template.

• Documentation
• Development tools (e.g. linter, formatter, etc.)
• Development container
• Global update (please specify)
• Other (please specify) : fix how registration gets used !

Describe your improvement

Conventions

Documented in nf-neuro/website#47 !

There has been a lot of confusion throughout the evolution of registration in the library, for very good reasons by the way, I'm not pointing fingers. Nomenclature around registration is still changing, formats are not universal and spaces are used differently between tools. This PR attempts to fix it for nf-neuro, so it makes some decisions :

• Global file formats : The ITK formats, aka rigid/affine in .mat/txt and deformation fields in .nii.gz.
  1. Modules should output in that format. If they can't, a conversion module is required.
  2. Subworkflows must output in that format, using the conversion modules if needed.
• Transformation order : Linear Algebra Convention, transformations are applied from right to left.
  • Registration tools must output transformation lists for images and tractograms following this ordering
  • Transformation tools must expect in input a transformation list following this ordering
• Separate transformation outputs : should be avoided when possible as they don't generalize, but they are needed.
  • Registration tools must output the first affine and the first warp computed (and their inverse) as separate outputs
  • Legacy modules that use them should be upgraded, but would require too much work to be included here
• Space order :
  • In registration : Fixed space comes before moving space.
    • Order of inputs : Fixed, Moving, Masks (optional), other uncommon and optional inputs
  • In application : Moving space comes before fixed space
    • Order of inputs : [images], reference, [transformations]

Major changes

• Upgrade all registration modules to abide to the conventions above
• Upgrade the registration subworkflow to always output ITK formatted transforms
• Upgrade all subworkflows to use the registration subworkflow
  • Bonus features : you can use synthmorph with bundleseg, and synthmorph and easyreg with output_template_space and tractoflow !
• Uniformize output filenames across modules

Minor changes

• Uniformize naming across the board (I might have missed some, I need to go back and check)
• Update all metadata for anything that touches registration
• Add possibility to specify choices as a yaml array in meta
• Reorder and reformat markdown for better lisibility (might want to do a little more in another PR)
• Improve tests names and create new cases or improve them when needed
• Improve configuration : use global nextflow.config (with minimally output publishing) + one in cases needing specific configuration

Describe how to test your improvement

Run lint and test for registration modules and the subworkflows that uses them

Checklist before requesting a review

• Ensure the syntax is correct (EditorConfig and Prettier must pass)
• Run the test suites if your changes affect any module
• Regenerate the Poetry lock file if you have updated the dependencies
• Ensure the documentation is up-to-date

[gagnonanthony]

It was long overdue, I like it! 😄 @AlexVCaron I am not aware of the offline discussion prompting these changes, but why using the forward{1,2} nomenclature? Is this a common format used elsewhere?

If we are to change how we name transformation files, we could also go for the BIDS nomenclature directly, which I think is pretty intuitive (e.g. from-T1w_to-dwi_{affine,warp}. Those are just thoughts however!

[AlexVCaron]

It was long overdue, I like it! 😄 @AlexVCaron I am not aware of the offline discussion prompting these changes, but why using the forward{1,2} nomenclature? Is this a common format used elsewhere?

If we are to change how we name transformation files, we could also go for the BIDS nomenclature directly, which I think is pretty intuitive (e.g. from-T1w_to-dwi_{affine,warp}. Those are just thoughts however!

Thank you ! The new nomenclature is homemade, I was just looking for a clear annotation for transform directions, "forward"+"backward" seemed better than ""+"inverse". I do like going all-BIDS though, hadn't thought of it ! Could you point me to the standard or specification ? I cannot find it in the v1.0.0 of the BIDS specification.

[AlexVCaron]

Fixes #174

[gagnonanthony]

Sadly, I am unsure if it is truly supported by BIDS as of now (derivative files naming is a bit experimental). I saw that post on NeuroStars that discusses it. I mainly adopted it through the examples I saw from fmriprep outputs 😅 .

However, from my understanding, the current convention is to specify the origin and destination (from-T1w_to-dwi) in addition to the type (from-T1w_to-dwi_desc-{affine,warp}.{mat,txt,nii.gz}).

[AlexVCaron]

Sadly, I am unsure if it is truly supported by BIDS as of now (derivative files naming is a bit experimental). I saw that post on NeuroStars that discusses it. I mainly adopted it through the examples I saw from fmriprep outputs 😅 .

However, from my understanding, the current convention is to specify the origin and destination (from-T1w_to-dwi) in addition to the type (from-T1w_to-dwi_desc-{affine,warp}.{mat,txt,nii.gz}).

Still in WIP, but we can use what we like in it and adapt for what we need. Here are the entities that would create the names :

• from and to cover fully the aspect of space
• desc to affine or warp covers fully the aspect of type
• index to cover the aspect of position of the transform in a the chain (my addition)
  • applied in reverse order to abide with linear algebra
• mode to cover what kind of transform, image or points (in the potential spec)

Would give something like this, for an affine+warp :

• Image :
  • sub-001_from-T1w_to-dwi_desc-affine_index-1_mode-image.mat
  • sub-001_from-T1w_to-dwi_desc-warp_index-0_mode-image.nii.gz
• Tractogram (transforms are inverted from image, only way to know is index+mode) :
  • sub-001_from-T1w_to-dwi_desc-affine_index-0_mode-points.mat
  • sub-001_from-T1w_to-dwi_desc-warp_index-1_mode-points.nii.gz

[gagnonanthony]

I am onboard with that! I would place the desc- section at the end of the filename (for consistency with the other files) unless it is specified that mode should be after. That way it should look like sub-001_from-T1w_to-dwi_index-1_mode-points_desc-warp.nii.gz.

[AlexVCaron]

The official convention is in PR : nf-neuro/website#47

The set of entities necessary to distinguish between forward and backward transformation sequences : (from or to) + mode. If you don't know from and to, then you need to know which transform is at which index. That's it !

Or is it ? There is a corner case in the above for Nextflow : when both from and to are variable and the number and types of transformation stages is variable, there are no clean or stable way of defining the glob pattern of process outputs, the code bloats up horribly and behaves badly.

The problem :

1. The script section isn't accessible to the output section, so we must parse the modality type inline
2. Definition outside the process scope are prohibited, so no helper function.
3. Collecting the modality type from the filename is lengthy and potentially unstable, unless we go full BIDS everywhere. Usually, we use ${fixed_image.name.tokenize(".")[0].tokenize("__")[1], which turns out does not work for most names in the library or with established conventions. Sometimes, the modality name is at the end, but sometimes we patch _processed or out_ or other things around it.
4. Each output definition bloats up, and becomes harder and harder to read.

It happens in registration/synthmorph, the only module for now that outputs 1+ transformations and for which neither the fixed or moving modality is known prior to execution. It would happen if I end up porting registration from versaFlow (5 stages total). It could also happen in a generic ants registration module with multi-stage transformations.

This may be a case of BIDS overhaul of nf-neuro, which should happen, but not in this PR. The logic could work well, but it needs well formatted filenames all-around. Maybe we should keep my custom forward/backward naming convention for now and orchestrate for BIDS more carefully ?

[gagnonanthony]

I agree, maybe abiding to the BIDS conventions should be done in all of nf-neuro at the same time. Otherwise, it can quickly become a mess of conventions...

The forward/backward naming convention seems fine for now, but I do like the from/to that BIDS proposes. Maybe we could brainstorm a bit on how we should implement BIDS all around?

[gdevenyi]

Lots of tiny commentary sprinkled in here.

Main thing is, I agree with the forward/backward naming here, there's a few more places to standardize the nomenclature regarding this.

Its a big PR, so I'll probably want to do another round.

[AlexVCaron]

@gdevenyi tests are passing with all naming changes, you can start your second review round !