[ENH] Support for multiple initial-moving-transforms for ants.Registration and ComposeMultiTransforms #2187
Conversation
|
@Gilles86 - regarding the reversing of transforms, there is already an output ( |
|
@Gilles86 - just checking in on the status of this. also a merge with current master would be advised. |
Codecov Report
@@ Coverage Diff @@
## master #2187 +/- ##
==========================================
+ Coverage 72.31% 72.32% +<.01%
==========================================
Files 1180 1181 +1
Lines 58879 58911 +32
Branches 8474 8482 +8
==========================================
+ Hits 42579 42607 +28
- Misses 14921 14923 +2
- Partials 1379 1381 +2
Continue to review full report at Codecov.
|
…ype into ants_compose_multitransform
|
Hey @satra, Thanks for the tip about the reverse_transforms. I misunderstood that output field for a list of inverse transforms! Maybe we should extend the documentation a bit? What do you think? I merged this branch with origin/master and made some new commits that make this branch pass all tests. Do we need anything else? |
|
@Gilles86 - if you could extend the documentation as you proposed that'd be awesome. Since you have been looking into this the latest, you are probably the best indicated candidate to do so :) |
|
I updated the documentation with some information I think is useful. |
There was a problem hiding this comment.
This is a great work. I'm +1 after a few changes to improve readability and exploit nipype's name_source are done.
Thanks a very much for this @Gilles86, happy to help if needed.
Also, there are some linting errors worth addressing in this PR:
flake8 nipype/interfaces/ants/registration.py --ignore=E501
nipype/interfaces/ants/registration.py:56:42: E127 continuation line over-indented for visual indent
nipype/interfaces/ants/registration.py:57:42: E127 continuation line over-indented for visual indent
nipype/interfaces/ants/registration.py:879:81: E261 at least two spaces before inline comment
nipype/interfaces/ants/registration.py:1041:10: E126 continuation line over-indented for hanging indent
nipype/interfaces/ants/registration.py:1044:10: E123 closing bracket does not match indentation of opening bracket's line
nipype/interfaces/ants/registration.py:1046:10: E126 continuation line over-indented for hanging indent
nipype/interfaces/ants/registration.py:1048:10: E123 closing bracket does not match indentation of opening bracket's line
nipype/interfaces/ants/registration.py:1050:10: E126 continuation line over-indented for hanging indent
nipype/interfaces/ants/registration.py:1052:10: E123 closing bracket does not match indentation of opening bracket's line
nipype/interfaces/ants/registration.py:1054:10: E126 continuation line over-indented for hanging indent
nipype/interfaces/ants/registration.py:1056:10: E123 closing bracket does not match indentation of opening bracket's line
nipype/interfaces/ants/registration.py:1058:10: E126 continuation line over-indented for hanging indent
nipype/interfaces/ants/registration.py:1060:10: E123 closing bracket does not match indentation of opening bracket's line
nipype/interfaces/ants/registration.py:1062:10: E126 continuation line over-indented for hanging indent
nipype/interfaces/ants/registration.py:1065:10: E123 closing bracket does not match indentation of opening bracket's line
nipype/interfaces/ants/registration.py:1067:10: E126 continuation line over-indented for hanging indent
nipype/interfaces/ants/registration.py:1071:10: E123 closing bracket does not match indentation of opening bracket's line
nipype/interfaces/ants/registration.py:1073:10: E126 continuation line over-indented for hanging indent
nipype/interfaces/ants/registration.py:1077:10: E123 closing bracket does not match indentation of opening bracket's line
nipype/interfaces/ants/registration.py:1081:9: E123 closing bracket does not match indentation of opening bracket's line
nipype/interfaces/ants/registration.py:1085:9: E123 closing bracket does not match indentation of opening bracket's line
nipype/interfaces/ants/registration.py:1121:9: E122 continuation line missing indentation or outdented
nipype/interfaces/ants/registration.py:1122:9: E122 continuation line missing indentation or outdented
nipype/interfaces/ants/registration.py:1136:13: E122 continuation line missing indentation or outdented
nipype/interfaces/ants/registration.py:1142:13: E122 continuation line missing indentation or outdented| @@ -421,8 +436,57 @@ class RegistrationOutputSpec(TraitedSpec): | |||
| class Registration(ANTSCommand): | |||
|
|
|||
| """ | |||
| `antsRegister <http://stnava.github.io/ANTs/>`_ registers a 'moving_image' to a 'fixed_image', | |||
There was a problem hiding this comment.
antsRegistration ?, also please replace single quotes "'" with tilted quotes "``" so that sphinx renders this as code (eg. ``fixed_image``).
| @@ -421,8 +436,57 @@ class RegistrationOutputSpec(TraitedSpec): | |||
| class Registration(ANTSCommand): | |||
|
|
|||
| """ | |||
| `antsRegister <http://stnava.github.io/ANTs/>`_ registers a 'moving_image' to a 'fixed_image', | |||
| using a predefined (sequence of) cost function(s) and transformation operattions. | |||
| Note, however, that ANTS tools always apply lists of transformations in reverse order (the last | ||
| transformation in the list is applied first). Therefore, if the output forward_transforms | ||
| is a list, one can not directly feed it into, for example, ants.ApplyTransforms. To | ||
| make ants.ApplyTransforms apply the transformations in the same order as ants.Registration, |
There was a problem hiding this comment.
``ants.ApplyTransforms`` and ``ants.Registration``
|
|
||
| Note, however, that ANTS tools always apply lists of transformations in reverse order (the last | ||
| transformation in the list is applied first). Therefore, if the output forward_transforms | ||
| is a list, one can not directly feed it into, for example, ants.ApplyTransforms. To |
| @@ -865,6 +985,22 @@ def _format_winsorize_image_intensities(self): | |||
| return '--winsorize-image-intensities [ %s, %s ]' % (self.inputs.winsorize_lower_quantile, | |||
| self.inputs.winsorize_upper_quantile) | |||
|
|
|||
| def _get_initial_transform_filenames(self): | |||
| retval = ['--initial-moving-transform'] | |||
There was a problem hiding this comment.
Perhaps more readable:
def _get_initial_transform_filenames(self):
n_transforms = len(self.inputs.initial_moving_transform)
# Assume transforms should not be inverted by default
invert_flags = [0] * n_transforms
if isdefined(self.inputs.invert_initial_moving_transform):
if len(self.inputs.invert_initial_moving_transform) != n_transforms:
raise Exception(
'Inputs "initial_moving_transform" and "invert_initial_moving_transform"'
'should have the same length.')
invert_flags = self.inputs.invert_initial_moving_transform
retval = ["[ %s, %d ]" % (xfm, int(flag)) for xfm, flag in zip(
self.inputs.initial_moving_transform, invert_flags)]
return " ".join(['--initial-moving-transform'] + retval)Please also note two details: the direct casting with int(flag) to avoid extra if .. else and the more specific error message when the invert flags don't match the number of input xforms.
|
|
||
|
|
||
| class ComposeMultiTransform(ANTSCommand): | ||
| """ |
There was a problem hiding this comment.
Please add a one-line description of what ComposeMultiTransform does.
nipype/interfaces/ants/utils.py
Outdated
| input_spec = ComposeMultiTransformInputSpec | ||
| output_spec = ComposeMultiTransformOutputSpec | ||
|
|
||
| def _format_arg(self, opt, spec, val): |
There was a problem hiding this comment.
This should not be necessary, please remove.
There was a problem hiding this comment.
If I don't include this I get an error:
traits.trait_errors.TraitError: Cannot set the undefined 'dimension' attribute of a 'ANTSCommandInputSpec' object.
/src/nipype/nipype/interfaces/ants/utils.py:256: UnexpectedException
Maybe this functionality is not yet implemented for ANTSCommand?
There was a problem hiding this comment.
I think that'd be fixed by removing mandatory=True from the dimension input trait.
There was a problem hiding this comment.
Yay! (it works on my settings at least 👍 )
There was a problem hiding this comment.
Cool! This works for me too. Maybe it was too late yesterday :).
nipype/interfaces/ants/utils.py
Outdated
| class ComposeMultiTransformInputSpec(ANTSCommandInputSpec): | ||
| dimension = traits.Enum(3, 2, argstr='%d', usedefault=True, mandatory=True, | ||
| position=0, desc='image dimension (2 or 3)') | ||
| output_transform = File(argstr='%s', mandatory=True, position=1, |
There was a problem hiding this comment.
Please use name_source:
output_transform = File(argstr='%s', mandatory=True, position=1,
name_source=['transforms'], name_template='%s_composed',
desc='the name of the resulting transform.')And remove member _list_outputs that will not be necessary anymore.
|
Thanks for the feedback, @oesteban! I think I implemented all of it. leaving out the input/outputspec-definition in the Interface-class doesn't work for me. Maybe you can have a look why this is. |
|
Sorry I forgot to update the specs. Hopefully this is the one passing. |
In my workflows I use multiple registration steps that I want to inspect and build upon each other. For that I think it would be useful if you can use multiple transforms to initialize ANTSRegistration and/or compose new transforms out of a list of transforms.
I tried to implement that in this pull request. It would be good if someone would review this code. The antsRegistration script and its interface are quite messy.
In a related note: the antsRegistration-interface currently outputs transforms in the order of which you would like to apply them. However, if you want to input them into ans.ApplyTransforms or, now, as initial_moving_transform for ants.Registration you should give them in the reverse order. I think that is quite unclear and not user-friendly. However, I can see that it's too late to reverse this now. Should ants.Registration maybe get an input flag which can inverse the transforms already for you?