WIP: ENH: Add BlockMatching registration Python wrappings + example by tbirdso · Pull Request #150 · KitwareMedical/ITKUltrasound

tbirdso · 2021-07-09T13:31:08Z

Adding wrappings and Python example to port block matching multi-resolution registration test from cxx. Wrappings will allow us to more rapidly iterate over registration parameter adjustments and take advantage of tools for examining intermediate results in Python.

Issues with runtime warnings to be discussed in comments below.

tbirdso · 2021-07-09T13:34:54Z

+# Wrap class hierarchy
+itk_wrap_include("itkImageToImageFilter.h")
+itk_wrap_class("itk::ImageToImageFilter" POINTER)
+  foreach(t ${WRAP_ITK_REAL})
+    foreach(d ${ITK_WRAP_IMAGE_DIMS})
+      itk_wrap_template("IIR${d}${d}I${ITKM_V${t}${d}}${d}"
+                        "itk::Image<itk::ImageRegion<${d}>,${d}>, itk::Image<${ITKT_V${t}${d}},${d}>")
+    endforeach()
+  endforeach()
+itk_end_wrap_class()


Seeing the following warnings when instantiating any BlockMatching class at runtime:

Template itk::ImageToImageFilter<itk::Image<itk::Vector<double,2>,2>,itk::Image<itk::CovariantVector<double,2>,2>>
already defined as <class 'itk.itkImageToImageFilterAPython.itkImageToImageFilterIVD22ICVD22'>
is redefined as {cl}
Warning: template already defined 'itk::ImageToImageFilter<itk::Image<itk::Vector<double,2>,2>,itk::Image<itk::CovariantVector<double,2>,2>>'
Template itk::ImageToImageFilter<itk::Image<itk::Vector<double,3>,3>,itk::Image<itk::CovariantVector<double,3>,3>>
already defined as <class 'itk.itkImageToImageFilterAPython.itkImageToImageFilterIVD33ICVD33'>
is redefined as {cl}
Warning: template already defined 'itk::ImageToImageFilter<itk::Image<itk::Vector<double,3>,3>,itk::Image<itk::CovariantVector<double,3>,3>>'

The warnings are introduced with these changes, but we are not explicitly wrapping ImageToImageFilter for these templates. Any thoughts on where this could be coming from?

😕 interesting. I do not see anything via inspection, either. 👁️ I will start a local build.

@thewtex Were you able to reproduce by any chance?

I did not reproduce these warnings locally.

I did get

itkBlockMatchingMultiResolutionImageRegistrationMethod: warning(4): ITK type not wrapped, or currently not known: itk::Image< itk::Point< double, 2 >, 2 > itkBlockMatchingMultiResolutionImageRegistrationMethod: warning(4): ITK type not wrapped, or currently not known: itk::Image< itk::Point< double, 3 >, 3 >

at build time.

And,

>>> itk.Ultrasound.NormalizedCrossCorrelationMetricImageFilter.New() Warning: Unknown parameter 'itk::SmartPointer<itk::Image<float,2>>' in template 'itk::Image' Warning: Unknown parameter 'itk::SmartPointer<itk::Image<float,3>>' in template 'itk::Image' Namespace already has a value for ImageRegistrationMethod, which is not an itkTemplate instance for class itk::BlockMatching::ImageRegistrationMethod. Overwriting old value. Namespace already has a value for MultiResolutionImageRegistrationMethod, which is not an itkTemplate instance for class itk::BlockMatching::MultiResolutionImageRegistrationMethod. Overwriting old value. <itk.ITKCommonBasePython.itkObject; proxy of <Swig Object of type 'itkObject *' at 0x7fa4d95d8660> >

at run time.

We could likely replace the SmartPointer's with raw pointers.

Interesting, thanks for taking a look. I'll try a clean build over the weekend and see if warnings still appear on my end.

My guess is that the point wrapping warnings are related to my having ITK_WRAP_DOUBLE enabled locally, I'll take a closer look.

We could likely replace the SmartPointer's with raw pointers.
Will do. This would occur within the cxx classes as well as the wrapping files, right?

tbirdso · 2021-07-09T13:40:30Z

+# Wrap base classes for itk::BlockMatching::MetricImageToDisplacementCalculator public members
+itk_wrap_class("itk::Image" POINTER)
+  foreach(t ${WRAP_ITK_REAL})
+    foreach(d ${ITK_WRAP_IMAGE_DIMS})
+      itk_wrap_template("P${ITKM_${t}}${d}${d}" "itk::Point<${ITKT_${t}},${d}>, ${d}")
+      itk_wrap_template("SPI${ITKM_${t}}${d}${d}" "itk::SmartPointer<itk::Image<${ITKT_${t}},${d}>>, ${d}")
+    endforeach()
+  endforeach()
+itk_end_wrap_class()


These Image wrappings are required for itkBlockMatchingMetricImageToDisplacementCalculator public members of type 'MetricImageImageType. However, even after adding this wrapping we're seeing warnings about unknown parameters:

Warning: Unknown parameter 'itk::SmartPointer<itk::Image<float,2>>' in template 'itk::Image'
Warning: Unknown parameter 'itk::SmartPointer<itk::Image<float,3>>' in template 'itk::Image'
Warning: Unknown parameter 'itk::SmartPointer<itk::Image<double,2>>' in template 'itk::Image'
Warning: template already defined 'itk::Image<itk::SmartPointer<itk::Image<double,2>>,2>'
Warning: Unknown parameter 'itk::SmartPointer<itk::Image<double,3>>' in template 'itk::Image'
Warning: template already defined 'itk::Image<itk::SmartPointer<itk::Image<double,3>>,3>'

This may be an issue with SmartPointer not being wrapped, will revisit shortly.

Attempting to wrap SmartPointer yields compile time errors. Could this be an issue with circular dependencies?

16>C:\repos\itkus\Wrapping\Modules\Ultrasound\itkBlockMatchingMultiResolutionImageRegistrationMethodPython.cpp(42678,30): error C3861: 'itkImageD2_cast': identifier not found
16>C:\repos\itkus\Wrapping\Modules\Ultrasound\itkBlockMatchingMultiResolutionImageRegistrationMethodPython.cpp(43971,16): error C3861: 'itkImageBase2_TransformPhysicalPointToIndex': identifier not found
16>C:\repos\itkus\Wrapping\Modules\Ultrasound\itkBlockMatchingMultiResolutionImageRegistrationMethodPython.cpp(43995,3): error C2065: 'itkContinuousIndexD2': undeclared identifier
16>C:\repos\itkus\Wrapping\Modules\Ultrasound\itkBlockMatchingMultiResolutionImageRegistrationMethodPython.cpp(43995,24): error C2146: syntax error: missing ';' before identifier 'result'
16>C:\repos\itkus\Wrapping\Modules\Ultrasound\itkBlockMatchingMultiResolutionImageRegistrationMethodPython.cpp(43995,24): error C2065: 'result': undeclared identifier
16>C:\repos\itkus\Wrapping\Modules\Ultrasound\itkBlockMatchingMultiResolutionImageRegistrationMethodPython.cpp(44037,7): error C2065: 'result': undeclared identifier
16>C:\repos\itkus\Wrapping\Modules\Ultrasound\itkBlockMatchingMultiResolutionImageRegistrationMethodPython.cpp(44037,16): error C3861: 'itkImageBase2_TransformPhysicalPointToContinuousIndex': identifier not found
16>C:\repos\itkus\Wrapping\Modules\Ultrasound\itkBlockMatchingMultiResolutionImageRegistrationMethodPython.cpp(44046,15): error C2061: syntax error: identifier 'itkContinuousIndexD2'
...

dzenanz

Looks good. I don't have advice for hunting down the warnings :(

thewtex

@tbirdso beautiful!!

thewtex · 2021-07-13T16:53:16Z

+# Wrap class hierarchy
+itk_wrap_include("itkImageToImageFilter.h")
+itk_wrap_class("itk::ImageToImageFilter" POINTER)
+  foreach(t ${WRAP_ITK_REAL})
+    foreach(d ${ITK_WRAP_IMAGE_DIMS})
+      itk_wrap_template("IIR${d}${d}I${ITKM_V${t}${d}}${d}"
+                        "itk::Image<itk::ImageRegion<${d}>,${d}>, itk::Image<${ITKT_V${t}${d}},${d}>")
+    endforeach()
+  endforeach()
+itk_end_wrap_class()


😕 interesting. I do not see anything via inspection, either. 👁️ I will start a local build.

tbirdso · 2021-07-16T17:36:13Z

Separated out some BlockMatching wrappings into individual files and added itkBlockMatchingBayesianRegularizationDisplacementCalculator.wrap.

thewtex · 2021-07-20T15:41:08Z

Great progress here.

Next steps appear to be

Rename classes to unique names when there is a conflict.
Change SmartPointer to raw pointers in classes if possible.

tbirdso · 2021-07-30T18:27:13Z

Wrapping succeeds and MultiResolutionImageRegistrationMethod Python example passes, but seeing a new CTest error:

28/33 Test #28: itkBlockMatchingBayesianRegularizationDisplacementCalculatorTest ......................***Exception: SegFault 7.91 sec

Debugging locally, the error happens here in itkBlockMatchingBayesianRegularizationDisplacementCalculator.hxx:

MetricImageImageIteratorType imageImageIt(this->m_MetricImageImage, region);
MetricImagePointerType image;
// We add eps so we don't take the log of zero, and having a probability of
// 0.0 is ... pessimistic;-)
const PixelType lowerBound = this->m_MetricLowerBound + NumericTraits::epsilon();
for (imageImageIt.GoToBegin(); !imageImageIt.IsAtEnd(); ++imageImageIt)
{
image = imageImageIt.Get();
MetricImageIteratorType it(image, image->GetLargestPossibleRegion());

Where the segfault is thrown when the test attempts to access metadata for the MetricImageType object image, which is not a nullptr.

Current hypothesis is that changing MetricImageImageType from holding smart pointers to MetricImageType objects to holding raw pointers instead causes reference counts to not be updated when we try to initialize each MetricImageImageType pixel with a smart pointer to an allocated metric image, so the memory buffer for each metric image is deallocated before it can be used. We can't rely on smart pointer pixel types here due to wrapping constraints and I assume defining each pixel directly as a MetricImage could severely impact performance.

@thewtex do you have thoughts on this?

thewtex · 2021-12-08T16:03:24Z

Current hypothesis is that changing MetricImageImageType from holding smart pointers to MetricImageType objects to holding raw pointers instead causes reference counts to not be updated

Yes, that sounds right -- we may have to revert that change.

tbirdso commented Jul 9, 2021

View reviewed changes

Comment thread wrapping/itkBlockMatchingMultiResolutionImageRegistrationMethod.wrap Outdated

tbirdso requested review from dzenanz and thewtex July 9, 2021 13:44

dzenanz approved these changes Jul 9, 2021

View reviewed changes

thewtex reviewed Jul 13, 2021

View reviewed changes

ENH: Add BlockMatching registration Python wrappings + example

3beeb9c

tbirdso force-pushed the wrap-blockmatching branch from f1f28bf to 3beeb9c Compare July 16, 2021 17:35

tbirdso added 5 commits July 26, 2021 13:07

WIP: Add image point wrapping for calculator member

d863da8

ENH: Rename BlockMatchingImageRegistrationMethod to mitigate collisions

025ad39

Merge branch 'wrap-blockmatching-renaming' into wrap-blockmatching

8ac535a

WIP: Replace MetricImage SmartPointer types with pointers for wrapping

4481861

WIP: Fix ImageImagePointerType typename issue

461879f

Conversation

tbirdso commented Jul 9, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dzenanz left a comment

Choose a reason for hiding this comment

Uh oh!

thewtex left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tbirdso commented Jul 16, 2021

Uh oh!

thewtex commented Jul 20, 2021

Uh oh!

tbirdso commented Jul 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thewtex commented Dec 8, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tbirdso commented Jul 30, 2021 •

edited

Loading