ITK icon indicating copy to clipboard operation
ITK copied to clipboard

ENH: Add Similarity3DTransform in itkLandmarkBasedTransformInitializer

Open PranjalSahu opened this issue 3 years ago • 7 comments

For adding iso-tropic scaling in the transformation matrix along with Rotation and Translation when using landmarks for transform initialization. Moved some common code from VersorRigid3DTransform and Similarity3DTransform and created ComputeCentroid and CreateMatrix. Introduced a PointType3D to handle the compilation issue when wrapping.

PR Checklist

  • [ ] No API changes were made (or the changes have been approved)
  • [ ] No major design changes were made (or the changes have been approved)
  • [X] Added test (or behavior not changed)
  • [ ] Updated API documentation (or API not changed)
  • [ ] Added license to new files (if any)
  • [ ] Added Python wrapping to new files (if any) as described in ITK Software Guide Section 9.5
  • [ ] Added ITK examples for all new major features (if any)

PranjalSahu avatar Jul 30 '22 22:07 PranjalSahu

Yes, there is some issue with the Python wrapping caused due to code refactoring. I am changing the .wrap file to account for it.

PranjalSahu avatar Aug 01 '22 15:08 PranjalSahu

Introduced a PointType3D to handle the compilation issue when wrapping.

PranjalSahu avatar Aug 01 '22 20:08 PranjalSahu

Yes I tested it locally with ctest. Doing one more test with a bigger dataset to check if it gives the correct output. With the smaller test sample, it gave the correct scaling output.

PranjalSahu avatar Aug 01 '22 20:08 PranjalSahu

As Matt is on vacation this week, let's not wait for his review. If @blowekamp does not review until 3-4PM, I say we merge.

dzenanz avatar Aug 02 '22 12:08 dzenanz

@dzenanz yes. I discovered one bug in mesh writing in binary mode in rc4.post2 that delayed the testing of this PR. I will confirm here once that is done.

PranjalSahu avatar Aug 02 '22 13:08 PranjalSahu

I have tested it using Python and there it is calling LandmarkBasedTransformInitializer with the VersorRigid3DTransform when using Similarity3DTransform. This is because Similarity3DTransform inherits from VersorRigid3DTransform. I will have to check how to handle this for Python wrappings.

PranjalSahu avatar Aug 03 '22 16:08 PranjalSahu

Changing the ordering of dynamic_cast and made it work in Python.

PranjalSahu avatar Aug 03 '22 16:08 PranjalSahu