elastix icon indicating copy to clipboard operation
elastix copied to clipboard

Document difference between `SetExternalInitialTransform` and `SetInitialTransform`

Open N-Dekker opened this issue 2 years ago • 5 comments
trafficstars

Both ElastixRegistrationMethod::SetExternalInitialTransform and ElastixRegistrationMethod::SetInitialTransform support specifying a "standard" ITK transform as initial transformation to an elastix registration. However, SetInitialTransform internally converts the specified transform to the corresponding elastix "advanced" transform, while SetExternalInitialTransform just uses a pointer to the specified ITK transform object during the registration process.

SetInitialTransform only works fine if there is a corresponding elastix "advanced" transform for the specified "standard" ITK transform. In practice this means that SetInitialTransform only supports the following standard ITK transform types:

  • itk::AffineTransform
  • itk::BSplineTransform
  • itk::Euler2D, itk::Euler3D
  • itk::Similarity2D, itk::Similarity3D
  • itk::TranslationTransform

On the other hand, SetExternalInitialTransform was designed to support itk::DisplacementFieldTransform.

If SetInitialTransform works for the specific ITK transform, it is preferred over SetExternalInitialTransform. (TODO Explain why: better performance? Jacobian/Hessian support?)

TODO: Move this to Doxygen, and if necessary, add a Jupyter Notebook.

N-Dekker avatar Sep 13 '23 17:09 N-Dekker

@N-Dekker a related observation while working with the API -- it seems that SetExternalInitialTransform and SetInitialTransform should take a const input?

thewtex avatar Sep 24 '23 23:09 thewtex

@thewtex

it seems that SetExternalInitialTransform and SetInitialTransform should take a const input?

Originally yes (meaning that originally both SetExternalInitialTransform and SetInitialTransform supported both const and non-const input). However, pull request https://github.com/SuperElastix/elastix/pull/949 commit 3b6d8ddb5fddadfcc6bcf5049a7523686505a89d (included with ITKElastix v0.18.1) has dropped support for setting const ExternalInitialTransform objects (unfortunately). This was necessary for ConvertToItkTransform. So now:

  • SetInitialTransform supports both const and non-const input
  • SetExternalInitialTransform only supports non-const input

Does that answer your question?

N-Dekker avatar Sep 25 '23 12:09 N-Dekker

I would expect that these methods do not modify their inputs, so they would be marked const. But are you saying that they may modify their inputs or the output shares modifiable memory with the input? Regarding this, is there something that could be added to the method documentation?

thewtex avatar Sep 26 '23 01:09 thewtex

I could indeed add a note, saying that a transform passed to SetExternalInitialTransform will not be modified by elastix (even though it must be non-const), thanks.

Rationale: ConvertToItkTransform allows converting an internal elastix specific "Combination Transform" to a non-const (modifiable) itk::CompositeTransform. If the internal elastix Combination Transform has an external initial transform (as one of its subtransforms), the itk::CompositeTransform returned by ConvertToItkTransform will have a pointer to the very same external initial transform (also as one of its subtransforms). So that's why the external initial transform is non-const.

If we would really need SetExternalInitialTransform to accept a pointer to a const transform object, we could either:

  • internally cast away the const (which is risky)
  • internally copy the content of the transform object, rather than just using a pointer to it (which may be time/memory consuming)

But I think the current approach (non-const-only) is OK.

N-Dekker avatar Sep 26 '23 07:09 N-Dekker

Note that at the moment ParameterObject.WriteParameterFiles is somewhat problematic, when one of the parameter maps contains an "ExternalTransform". The pointer to the external transform may be dangling. As I mentioned at https://github.com/InsightSoftwareConsortium/ITKElastix/issues/246#issuecomment-1764917427

N-Dekker avatar Oct 16 '23 17:10 N-Dekker