Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

2D similarity transform parameter map doesn't keep "CenterOfRotationPoint" parameter #93

Closed
NHPatterson opened this issue Dec 16, 2020 · 9 comments
Assignees

Comments

@NHPatterson
Copy link

Hi all, thanks for putting together this package, I'm really enjoying the ease of installation.

It seems the similarity transform as wrapped in ITKElastix doesn't return the CenterOfRotationPoint transform parameter (although it is written to file) when doing 2D to 2D registration.

I encountered this issue using SimpleElastix as well so it may be in elastix codebase itself.

import numpy as np
import itk

zero_image = np.zeros([1024,1024])

similarity_parameter_map = parameter_object.GetDefaultParameterMap("rigid")

# set method to similarity & 2D
similarity_parameter_map["Transform"] = ["SimilarityTransform"]
similarity_parameter_map["MovingImageDimension"] = ["2"]
similarity_parameter_map["FixedImageDimension"] = ["2"]

parameter_object = itk.ParameterObject.New()
parameter_object.AddParameterMap(similarity_parameter_map)

sim_result_image, sim_params = itk.elastix_registration_method(
    zero_image.astype(np.float32),
    zero_image.astype(np.float32),
    parameter_object=parameter_object,
    log_to_console=True,
    output_directory="./transforms_similarity",
)

transform_cent_rot = np.array(
    sim_params.GetParameter(0, "CenterOfRotationPoint")
).astype(np.float32)[::-1]

assert len(transform_cent_rot) == 2

print(sim_params)

ParameterObject (0000028751D7CE00)
  RTTI typeinfo:   class elastix::ParameterObject
  Reference Count: 1
  Modified Time: 2007229
  Debug: Off
  Object Name: 
  Observers: 
    none
ParameterMap 0: 
  (CenterOfRotationPoint)
  (CompressResultImage "false")
  (DefaultPixelValue 0)
  (Direction 1 0 0 1)
  (FinalBSplineInterpolationOrder 3)
  (FixedImageDimension 2)
  (FixedInternalImagePixelType "float")
  (HowToCombineTransforms "Compose")
  (Index 0 0)
  (InitialTransformParametersFileName "NoInitialTransform")
  (MovingImageDimension 2)
  (MovingInternalImagePixelType "float")
  (NumberOfParameters 4)
  (Origin 0 0)
  (ResampleInterpolator "FinalBSplineInterpolator")
  (Resampler "DefaultResampler")
  (ResultImageFormat "nii")
  (ResultImagePixelType "float")
  (Size 1024 1024)
  (Spacing 1 1)
  (Transform "SimilarityTransform")
  (TransformParameters 1 0 0 0)
  (UseDirectionCosines "true")

file on disk

(Transform "SimilarityTransform")
(NumberOfParameters 4)
(TransformParameters 1.000000 0.000000 0.000000 0.000000)
(InitialTransformParametersFileName "NoInitialTransform")
(UseBinaryFormatForTransformationParameters "false")
(HowToCombineTransforms "Compose")

// Image specific
(FixedImageDimension 2)
(MovingImageDimension 2)
(FixedInternalImagePixelType "float")
(MovingInternalImagePixelType "float")
(Size 1024 1024)
(Index 0 0)
(Spacing 1.0000000000 1.0000000000)
(Origin 0.0000000000 0.0000000000)
(Direction 1.0000000000 0.0000000000 0.0000000000 1.0000000000)
(UseDirectionCosines "true")

// SimilarityTransform specific
(CenterOfRotationPoint 511.5000000000 511.5000000000)

// ResampleInterpolator specific
(ResampleInterpolator "FinalBSplineInterpolator")
(FinalBSplineInterpolationOrder 3)

// Resampler specific
(Resampler "DefaultResampler")
(DefaultPixelValue 0.000000)
(ResultImageFormat "nii")
(ResultImagePixelType "float")
(CompressResultImage "false")
@NHPatterson NHPatterson changed the title 2D similarity transform wrapping does not wrap "CenterOfRotationPoint" 2D similarity transform parameter map doesn't keep "CenterOfRotationPoint" parameter Dec 16, 2020
@N-Dekker
Copy link
Collaborator

@NHPatterson Thanks for reporting this issue, Heath. Stefan (@stefanklein), Viktor (@ViktorvdValk) and I just discussed the issue. It appears that elastix::SimilarityTransformElastix should override CreateTransformParametersMap, in order to fix this issue. I will try to fix it soon!

@ViktorvdValk
Copy link
Collaborator

ViktorvdValk commented Dec 22, 2020

The same counts for AffineDTITransform, see issue 366 on elastix repo

@N-Dekker
Copy link
Collaborator

The same counts for AffineDTITransform, see issue 366 on elastix repo

Thanks @ViktorvdValk I just assigned issue SuperElastix/elastix#366 to myself 😄

N-Dekker added a commit to SuperElastix/elastix that referenced this issue Dec 22, 2020
Added `SimilarityTransformElastix<TElastix>::CreateTransformParametersMap` override, which adds the "CenterOfRotationPoint" parameter.

Addresses ITKElastix issue 93: "2D similarity transform parameter map doesn't keep "CenterOfRotationPoint" parameter", by Heath Patterson, 16 December, 2020.
InsightSoftwareConsortium/ITKElastix#93

Bug fixed with help from Stefan Klein and Viktor van der Valk.
N-Dekker added a commit to SuperElastix/elastix that referenced this issue Dec 22, 2020
Added `AffineDTITransformElastix<TElastix>::CreateTransformParametersMap` override, which adds the "CenterOfRotationPoint" parameter.

Addresses ITKElastix issue 93: "AffineDTITransform does not return the CenterOfRotationPoint", by dyliu2016, 27 November, 2020.
InsightSoftwareConsortium/ITKElastix#93

Bug fixed with help from Stefan Klein and Viktor van der Valk.
@N-Dekker
Copy link
Collaborator

@stefanklein @ViktorvdValk I think this issue should be fixed by SuperElastix/elastix#379 But then, what about the following three elastix transform types?

  • AdvancedAffineTransformElastix
  • AffineLogTransformElastix
  • EulerStackTransform

Are these three fine, or should they get a similar fix? Is it possible to check that in Python?

@stefanklein
Copy link
Collaborator

stefanklein commented Dec 22, 2020 via email

@ViktorvdValk
Copy link
Collaborator

@N-Dekker I checked all the transforms in python and the same happens for:

  • AffineLogStackTransform
  • AffineLogTransformElastix
  • EulerStackTransform

AdvancedAffineTransformElastix is just the AffineTransform which does return the CenterOfRotationPoint

@N-Dekker
Copy link
Collaborator

@ViktorvdValk Do you mean that those three transforms (AffineLogStackTransform, AffineLogTransformElastix, EulerStackTransform) also need to be fixed (the same like Similarity and AffineDTI)?

@ViktorvdValk
Copy link
Collaborator

Yes, to be certain, you check this with the verification Stefan mentioned, but if I print the output of one of these transforms, it doesn't contain the 'CenterOfRotationPoint' parameter.

N-Dekker added a commit to SuperElastix/elastix that referenced this issue Dec 23, 2020
Added `AffineDTITransformElastix<TElastix>::CreateTransformParametersMap` override, which adds the "CenterOfRotationPoint" parameter.

Addresses ITKElastix issue 93: "AffineDTITransform does not return the CenterOfRotationPoint", by dyliu2016, 27 November, 2020.
InsightSoftwareConsortium/ITKElastix#93

Bug fixed with help from Stefan Klein and Viktor van der Valk.
N-Dekker added a commit to SuperElastix/elastix that referenced this issue Dec 23, 2020
Added `SimilarityTransformElastix<TElastix>::CreateTransformParametersMap` override, which adds the "CenterOfRotationPoint" parameter.

Addresses ITKElastix issue 93: "2D similarity transform parameter map doesn't keep "CenterOfRotationPoint" parameter", by Heath Patterson, 16 December, 2020.
InsightSoftwareConsortium/ITKElastix#93

Bug fixed with help from Stefan Klein and Viktor van der Valk.
N-Dekker added a commit to SuperElastix/elastix that referenced this issue Dec 23, 2020
Added `AffineDTITransformElastix<TElastix>::CreateTransformParametersMap` override, which adds the "CenterOfRotationPoint" parameter.

Addresses ITKElastix issue 93: "AffineDTITransform does not return the CenterOfRotationPoint", by dyliu2016, 27 November, 2020.
InsightSoftwareConsortium/ITKElastix#93

Bug fixed with help from Stefan Klein and Viktor van der Valk.
N-Dekker added a commit to SuperElastix/elastix that referenced this issue Dec 25, 2020
Added `SimilarityTransformElastix<TElastix>::CreateTransformParametersMap` override, which adds the "CenterOfRotationPoint" parameter.

Addresses ITKElastix issue 93: "2D similarity transform parameter map doesn't keep "CenterOfRotationPoint" parameter", by Heath Patterson, 16 December, 2020.
InsightSoftwareConsortium/ITKElastix#93

Bug fixed with help from Stefan Klein and Viktor van der Valk.
N-Dekker added a commit to SuperElastix/elastix that referenced this issue Dec 25, 2020
Added `AffineDTITransformElastix<TElastix>::CreateTransformParametersMap` override, which adds the "CenterOfRotationPoint" parameter.

Addresses ITKElastix issue 93: "AffineDTITransform does not return the CenterOfRotationPoint", by dyliu2016, 27 November, 2020.
InsightSoftwareConsortium/ITKElastix#93

Bug fixed with help from Stefan Klein and Viktor van der Valk.
N-Dekker added a commit to SuperElastix/elastix that referenced this issue Dec 25, 2020
…k, EulerStack, Similarity

Added `CustomizeTransformParametersMap` overrides, which add the missing "CenterOfRotationPoint" parameter to:

 - `AffineDTITransformElastix`
 - `AffineLogTransformElastix`
 - `AffineLogStackTransform`
 - `EulerStackTransform`
 - `SimilarityTransformElastix`.

Addresses the following issues:

ITKElastix issue 93: "2D similarity transform parameter map doesn't keep "CenterOfRotationPoint" parameter", by Heath Patterson, 16 December, 2020.
InsightSoftwareConsortium/ITKElastix#93

SuperElastix/elastix issue 366: "AffineDTITransform does not return the CenterOfRotationPoint", by dyliu2016, 27 November, 2020
#366

Fixed with help from Stefan Klein and Viktor van der Valk.
N-Dekker added a commit to SuperElastix/elastix that referenced this issue Dec 25, 2020
Added `CustomizeTransformParametersMap` overrides, which add the missing "CenterOfRotationPoint" parameter to:

 - `AffineDTITransformElastix`
 - `AffineLogTransformElastix`
 - `AffineLogStackTransform`
 - `EulerStackTransform`
 - `SimilarityTransformElastix`.

Addresses the following issues:

ITKElastix issue 93: "2D similarity transform parameter map doesn't keep "CenterOfRotationPoint" parameter", by Heath Patterson, 16 December, 2020.
InsightSoftwareConsortium/ITKElastix#93

SuperElastix/elastix issue 366: "AffineDTITransform does not return the CenterOfRotationPoint", by dyliu2016, 27 November, 2020
#366

Fixed with help from Stefan Klein and Viktor van der Valk.
N-Dekker added a commit to SuperElastix/elastix that referenced this issue Dec 26, 2020
Added `CustomizeTransformParametersMap` overrides, which add the missing "CenterOfRotationPoint" parameter to:

 - `AffineDTITransformElastix`
 - `AffineLogTransformElastix`
 - `AffineLogStackTransform`
 - `EulerStackTransform`
 - `SimilarityTransformElastix`.

Addresses the following issues:

ITKElastix issue 93: "2D similarity transform parameter map doesn't keep "CenterOfRotationPoint" parameter", by Heath Patterson, 16 December, 2020.
InsightSoftwareConsortium/ITKElastix#93

SuperElastix/elastix issue 366: "AffineDTITransform does not return the CenterOfRotationPoint", by dyliu2016, 27 November, 2020
#366

Fixed with help from Stefan Klein and Viktor van der Valk.
N-Dekker added a commit to SuperElastix/elastix that referenced this issue Dec 26, 2020
Added `CustomizeTransformParametersMap` overrides, which add the missing "CenterOfRotationPoint" parameter to:

 - `AffineDTITransformElastix`
 - `AffineLogTransformElastix`
 - `AffineLogStackTransform`
 - `EulerStackTransform`
 - `SimilarityTransformElastix`.

Addresses the following issues:

ITKElastix issue 93: "2D similarity transform parameter map doesn't keep "CenterOfRotationPoint" parameter", by Heath Patterson, 16 December, 2020.
InsightSoftwareConsortium/ITKElastix#93

SuperElastix/elastix issue 366: "AffineDTITransform does not return the CenterOfRotationPoint", by dyliu2016, 27 November, 2020
#366

Fixed with help from Stefan Klein and Viktor van der Valk.
N-Dekker added a commit to SuperElastix/elastix that referenced this issue Dec 28, 2020
Added `CreateDerivedTransformParametersMap` overrides, which add the missing "CenterOfRotationPoint" parameter to:

 - `AffineDTITransformElastix`
 - `AffineLogTransformElastix`
 - `AffineLogStackTransform`
 - `EulerStackTransform`
 - `SimilarityTransformElastix`.

Addresses the following issues:

ITKElastix issue 93: "2D similarity transform parameter map doesn't keep "CenterOfRotationPoint" parameter", by Heath Patterson, 16 December, 2020.
InsightSoftwareConsortium/ITKElastix#93

SuperElastix/elastix issue 366: "AffineDTITransform does not return the CenterOfRotationPoint", by dyliu2016, 27 November, 2020
#366

Fixed with help from Stefan Klein and Viktor van der Valk.
N-Dekker added a commit to SuperElastix/elastix that referenced this issue Dec 28, 2020
Added `CreateDerivedTransformParametersMap` overrides, which add the missing "CenterOfRotationPoint" parameter to:

 - `AffineDTITransformElastix`
 - `AffineLogTransformElastix`
 - `AffineLogStackTransform`
 - `EulerStackTransform`
 - `SimilarityTransformElastix`.

Addresses the following issues:

ITKElastix issue 93: "2D similarity transform parameter map doesn't keep "CenterOfRotationPoint" parameter", by Heath Patterson, 16 December, 2020.
InsightSoftwareConsortium/ITKElastix#93

SuperElastix/elastix issue 366: "AffineDTITransform does not return the CenterOfRotationPoint", by dyliu2016, 27 November, 2020
#366

Fixed with help from Stefan Klein and Viktor van der Valk.
N-Dekker added a commit to SuperElastix/elastix that referenced this issue Dec 31, 2020
Added `CreateDerivedTransformParametersMap` overrides, which add missing parameters to:

 - AffineDTITransformElastix
 - AffineLogStackTransform
 - AffineLogTransformElastix
 - BSplineTransformWithDiffusion
 - DeformationFieldTransform
 - EulerStackTransform
 - MultiBSplineTransformWithNormal
 - SimilarityTransformElastix
 - SplineKernelTransform
 - TranslationStackTransform
 - WeightedCombinationTransformElastix

Addresses the following issues:

ITKElastix issue 93: "2D similarity transform parameter map doesn't keep "CenterOfRotationPoint" parameter", by Heath Patterson, 16 December, 2020.
InsightSoftwareConsortium/ITKElastix#93

SuperElastix/elastix issue 366: "AffineDTITransform does not return the CenterOfRotationPoint", by dyliu2016, 27 November, 2020
#366

Declared `TransformBase::CreateDerivedTransformParametersMap()` pure virtual (`= 0`). Added an empty override to `TranslationTransformElastix`, just to keep allowing the transform to be instantiated.

Extended `CreateTransformParametersMapForDefaultTransform` GoogleTest unit test.

Note: Some of the overrides may still be incomplete, as indicated by "TODO" comments.
N-Dekker added a commit to SuperElastix/elastix that referenced this issue Jan 1, 2021
Added `CreateDerivedTransformParametersMap` overrides, which add missing parameters to:

 - AffineDTITransformElastix
 - AffineLogStackTransform
 - AffineLogTransformElastix
 - BSplineTransformWithDiffusion
 - DeformationFieldTransform
 - EulerStackTransform
 - MultiBSplineTransformWithNormal
 - SimilarityTransformElastix
 - SplineKernelTransform
 - TranslationStackTransform
 - WeightedCombinationTransformElastix

Addresses the following issues:

ITKElastix issue 93: "2D similarity transform parameter map doesn't keep "CenterOfRotationPoint" parameter", by Heath Patterson, 16 December, 2020.
InsightSoftwareConsortium/ITKElastix#93

SuperElastix/elastix issue 366: "AffineDTITransform does not return the CenterOfRotationPoint", by dyliu2016, 27 November, 2020
#366

Declared `TransformBase::CreateDerivedTransformParametersMap()` pure virtual (`= 0`). Added an empty override to `TranslationTransformElastix`, just to keep allowing the transform to be instantiated.

Extended `CreateTransformParametersMapForDefaultTransform` GoogleTest unit test.

Note: Some of the overrides may still be incomplete, as indicated by "TODO" comments.
N-Dekker added a commit to SuperElastix/elastix that referenced this issue Jan 2, 2021
Added `CreateDerivedTransformParametersMap` overrides, which add missing parameters to:

 - AffineDTITransformElastix
 - AffineLogStackTransform
 - AffineLogTransformElastix
 - BSplineTransformWithDiffusion
 - DeformationFieldTransform
 - EulerStackTransform
 - MultiBSplineTransformWithNormal
 - SimilarityTransformElastix
 - SplineKernelTransform
 - TranslationStackTransform
 - WeightedCombinationTransformElastix

Addresses the following issues:

ITKElastix issue 93: "2D similarity transform parameter map doesn't keep "CenterOfRotationPoint" parameter", by Heath Patterson, 16 December, 2020.
InsightSoftwareConsortium/ITKElastix#93

SuperElastix/elastix issue 366: "AffineDTITransform does not return the CenterOfRotationPoint", by dyliu2016, 27 November, 2020
#366

Declared `TransformBase::CreateDerivedTransformParametersMap()` pure virtual (`= 0`). Added an empty override to `TranslationTransformElastix`, just to keep allowing the transform to be instantiated.

Extended `CreateTransformParametersMapForDefaultTransform` GoogleTest unit test.

Note: Some of the overrides may still be incomplete, as indicated by "TODO" comments.
N-Dekker added a commit to SuperElastix/elastix that referenced this issue Jan 5, 2021
Added `CreateDerivedTransformParametersMap` overrides, which add missing parameters to:

 - AffineDTITransformElastix
 - AffineLogStackTransform
 - AffineLogTransformElastix
 - BSplineTransformWithDiffusion
 - DeformationFieldTransform
 - EulerStackTransform
 - MultiBSplineTransformWithNormal
 - SimilarityTransformElastix
 - SplineKernelTransform
 - TranslationStackTransform
 - WeightedCombinationTransformElastix

Addresses the following issues:

ITKElastix issue 93: "2D similarity transform parameter map doesn't keep "CenterOfRotationPoint" parameter", by Heath Patterson, 16 December, 2020.
InsightSoftwareConsortium/ITKElastix#93

SuperElastix/elastix issue 366: "AffineDTITransform does not return the CenterOfRotationPoint", by dyliu2016, 27 November, 2020
#366

Declared `TransformBase::CreateDerivedTransformParametersMap()` pure virtual (`= 0`). Added an empty override to `TranslationTransformElastix`, just to keep allowing the transform to be instantiated.

Extended `CreateTransformParametersMapForDefaultTransform` GoogleTest unit test.

Note: Some of the overrides may still be incomplete, as indicated by comments, saying "TODO If necessary, add possibly missing parameters".
N-Dekker added a commit to SuperElastix/elastix that referenced this issue Jan 5, 2021
Added `CreateDerivedTransformParametersMap` overrides, which add missing parameters to:

 - AffineDTITransformElastix
 - AffineLogStackTransform
 - AffineLogTransformElastix
 - BSplineTransformWithDiffusion
 - DeformationFieldTransform
 - EulerStackTransform
 - MultiBSplineTransformWithNormal
 - SimilarityTransformElastix
 - SplineKernelTransform
 - TranslationStackTransform
 - WeightedCombinationTransformElastix

Addresses the following issues:

ITKElastix issue 93: "2D similarity transform parameter map doesn't keep "CenterOfRotationPoint" parameter", by Heath Patterson, 16 December, 2020.
InsightSoftwareConsortium/ITKElastix#93

SuperElastix/elastix issue 366: "AffineDTITransform does not return the CenterOfRotationPoint", by dyliu2016, 27 November, 2020
#366

Declared `TransformBase::CreateDerivedTransformParametersMap()` pure virtual (`= 0`). Added an empty override to `TranslationTransformElastix`, just to keep allowing the transform to be instantiated.

Extended `CreateTransformParametersMapForDefaultTransform` GoogleTest unit test.

Note: Some of the overrides may still be incomplete, as indicated by comments, saying "TODO If necessary, add possibly missing parameters".
@N-Dekker
Copy link
Collaborator

N-Dekker commented Jan 5, 2021

@NHPatterson Hi Heath! I'm closing the issue now, as I think we got it fixed 😃 By pull request SuperElastix/elastix#379 "Add missing parameters to transform parameter maps", which is just merged onto the https://github.com/SuperElastix/elastix develop branch. If you have the time, please check it out! And of course, if you think the issue is not entirely fixed, please feel free to reopen the issue, or submit another one. Thanks again!

@N-Dekker N-Dekker closed this as completed Jan 5, 2021
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

No branches or pull requests

4 participants