PERF: Use FixedArray for BSplineInterpolationWeightFunction OutputType by N-Dekker · Pull Request #2712 · InsightSoftwareConsortium/ITK

N-Dekker · 2021-09-03T08:05:49Z

Replaced itk::Array by itk::FixedArray as output type (array of
weights) of itk::BSplineInterpolationWeightFunction.

Observed a significant improvement of the runtime performance of a call to itk::BSplineBaseTransform::TransformPoint: the runtime duration appeared ~30% lower than before, from 1.5 sec. down to 1.0 sec. when calling TransformPoint 10'000'000 times. (Tested using Visual C++ 2019 Release).

N-Dekker · 2021-09-03T08:45:54Z

Does anyone have a suggestion? https://open.cdash.org/viewBuildError.php?type=1&buildid=7436933 (Linux-Build5098-PR2712-BSplineInterpolationWeightFunction-FixedArray-Python) says:

itkBSplineInterpolationWeightFunction: warning(4): ITK type not wrapped, or currently not known: itk::FunctionBase< itk::ContinuousIndex< double, 2 >, itk::FixedArray< double, 9 > >

Wrapping/Typedefs/itkBSplineInterpolationWeightFunction.i:163: Warning 401: Maybe you forgot to instantiate 'itk::FunctionBase< itk::ContinuousIndex< double,3 >,itk::FixedArray< double,64 > >' using %template.

dzenanz

itk::FixedArray< double, 9 >

I think fixed arrays etc are wrapped by default only up to dimension 4.

Modules/Registration/Common/include/itkImageToImageMetric.hxx

N-Dekker · 2021-09-03T15:04:51Z

@dzenanz

I think fixed arrays etc are wrapped by default only up to dimension 4.

Would it be OK to wrap FixedArray<double, 9> and FixedArray<double, 64> as well, by default? If so, can you please tell me how to do that?

I think that both FunctionBase<ContinuousIndex<double, 2>, FixedArray<double, 9>> and FunctionBase<ContinuousIndex<double, 3>, FixedArray<double, 64>> would then also need to be wrapped, but I guess that should be sufficient.

dzenanz · 2021-09-03T15:42:38Z

I am not sure whether anything needs to be done for FunctionBase, but if it does it should probably be done here.

FixedArray.wrap is incredibly simple, and I don't know where WRAPPER_TEMPLATES is used. https://itk.org/ITKSoftwareGuide/html/Book1/ITKSoftwareGuide-Book1ch9.html does not shine light on it.

thewtex · 2021-09-04T00:17:46Z

FixedArray wrapping types are specified here:

ITK/Wrapping/WrapITKTypes.cmake

Lines 115 to 152 in 34f3db8

    
           WRAP_TYPE("itk::FixedArray" "FA" "itkFixedArray.h") 
        
             set(dims ${ITK_WRAP_IMAGE_DIMS_INCREMENTED}) 
        
             foreach(d ${ITK_WRAP_IMAGE_DIMS}) 
        
               math(EXPR d2 "${d} * 2") 
        
               # for itk::SymmetricSecondRankTensor 
        
               math(EXPR d3 "${d} * (${d} + 1) / 2") 
        
               list(APPEND dims ${d2} ${d3}) 
        
             endforeach() 
        
             # FixedArray can be used with dimensions describing 
        
             # image dimensions or vector components. Therefore it 
        
             # needs to have defined sizes for: 
        
             # - ITK_WRAP_IMAGE_DIMS_INCREMENTED 
        
             # - ITK_WRAP_VECTOR_COMPONENTS_INCREMENTED 
        
             # Dimensions 1;2;3;4;6 should always be wrapped. 
        
             UNIQUE(array_sizes "${dims};1;2;3;4;6;${ITK_WRAP_VECTOR_COMPONENTS_INCREMENTED}") 
        
             # 3-D FixedArrays are required as superclass of rgb pixels 
        
             # TODO: Do we need fixed arrays for all of these types? For just the selected 
        
             # pixel types plus some few basic cases? Or just for a basic set of types? 
        
             foreach(d ${array_sizes}) 
        
               ADD_TEMPLATE("${ITKM_D}${d}"  "${ITKT_D},${d}") 
        
               ADD_TEMPLATE("${ITKM_F}${d}"  "${ITKT_F},${d}") 
        
               ADD_TEMPLATE("${ITKM_UL}${d}" "${ITKT_UL},${d}") 
        
               ADD_TEMPLATE("${ITKM_ULL}${d}" "${ITKT_ULL},${d}") 
        
               ADD_TEMPLATE("${ITKM_US}${d}" "${ITKT_US},${d}") 
        
               ADD_TEMPLATE("${ITKM_UC}${d}" "${ITKT_UC},${d}") 
        
               ADD_TEMPLATE("${ITKM_UI}${d}" "${ITKT_UI},${d}") 
        
               ADD_TEMPLATE("${ITKM_SL}${d}" "${ITKT_SL},${d}") 
        
               ADD_TEMPLATE("${ITKM_SLL}${d}" "${ITKT_SLL},${d}") 
        
               ADD_TEMPLATE("${ITKM_SS}${d}" "${ITKT_SS},${d}") 
        
               ADD_TEMPLATE("${ITKM_SC}${d}" "${ITKT_SC},${d}") 
        
               ADD_TEMPLATE("${ITKM_B}${d}"  "${ITKT_B},${d}") 
        
             endforeach() 
        
           END_WRAP_TYPE() 
        
           set(itk_Wrap_FixedArray ${WRAPPER_TEMPLATES})

N-Dekker · 2021-09-04T10:54:21Z

FixedArray wrapping types are specified here:

@thewtex Thanks Matt! Please check my attempt to wrap FixedArray<double, 9> and FixedArray<double, 64>, with this PR:

ITK/Wrapping/WrapITKTypes.cmake

Lines 152 to 155 in 7c79455

    
           # Wrap FixedArray for BSplineInterpolationWeightFunction<CoordRep, 2, 2> and 
        
           # BSplineInterpolationWeightFunction<CoordRep, 3, 3>: 
        
           ADD_TEMPLATE("${ITKM_D}9" "${ITKT_D},9") 
        
           ADD_TEMPLATE("${ITKM_D}64" "${ITKT_D},64")

Unfortunately Darwin-Build5152-PR2712-BSplineInterpolationWeightFunction-FixedArray-Python still says:

itkBSplineInterpolationWeightFunction: warning(4): ITK type not wrapped, or currently not known: itk::FunctionBase< itk::ContinuousIndex< double, 2 >, itk::FixedArray< double, 9 > >
itkBSplineInterpolationWeightFunction: warning(4): ITK type not wrapped, or currently not known: itk::FunctionBase< itk::ContinuousIndex< double, 3 >, itk::FixedArray< double, 64 > >

Do you have more suggestions?

dzenanz · 2021-09-04T11:51:52Z

I guess we need to add something to https://github.com/InsightSoftwareConsortium/ITK/blob/master/Modules/Core/Common/wrapping/itkFunctionBase.wrap. I am not quite certain what.

N-Dekker · 2021-09-04T22:09:42Z

I guess we need to add something to https://github.com/InsightSoftwareConsortium/ITK/blob/master/Modules/Core/Common/wrapping/itkFunctionBase.wrap. I am not quite certain what.

@dzenanz Thanks! I'm not certain at all, but I gave it a try:

itk_wrap_template("${ITKM_CID2}${ITKM_FAD9}" "${ITKT_CID2}, ${ITKT_FAD9}") 
itk_wrap_template("${ITKM_CID3}${ITKM_FAD64}" "${ITKT_CID3}, ${ITKT_FAD64}")

ITK/Modules/Core/Common/wrapping/itkFunctionBase.wrap

Lines 29 to 31 in 90b6738

    
           # Required by BSplineInterpolationWeightFunction 
        
           itk_wrap_template("${ITKM_CID2}${ITKM_FAD9}" "${ITKT_CID2}, ${ITKT_FAD9}") 
        
           itk_wrap_template("${ITKM_CID3}${ITKM_FAD64}" "${ITKT_CID3}, ${ITKT_FAD64}")

I'd be surprised if it would work directly, but we'll see.

Replaced `itk::Array` by `itk::FixedArray` as output type (`OutputType`, `WeightsType`) of `itk::BSplineInterpolationWeightFunction`. Observed ~30% reduction of the runtime duration of a call to `BSplineBaseTransform::TransformPoint`: Going from 1.5 sec. down to 1.0 sec. for 10'000'000 `TransformPoint` function calls. Tested on Windows 10, Visual C++ 2019 64-bit, Release configuration. Allowed `BSplineInterpolationWeightFunction` to be wrapped, by adding extra `FixedArray` and `FunctionBase` wrappings.

Replaced `itk::Array` by `itk::FixedArray` as `ParameterIndexArrayType` of `itk::BSplineBaseTransform`. This is the type of indices, used during BSpline transformations. Suggested by Dženan Zukić. Observed more than 60% reduction of the runtime duration of a call to `BSplineBaseTransform::TransformPoint`: Going from 1.0 sec. down to less than 0.4 sec. for 10'000'000 `TransformPoint` function calls. Tested on Windows 10, Visual C++ 2019 64-bit, Release configuration.

Wrapping/WrapITKTypes.cmake

N-Dekker · 2021-09-07T15:01:32Z

@dzenanz @thewtex This PR builds now for the very first time 🎉 No more wrapping warnings! 😄

thewtex

@N-Dekker awesome work! 🥇 🐍 🪐

Merging for 5.3 RC1

CC @vicory @tbirdso

Replaced `itk::Array2D` by `itk::FixedArray`, used as private `TableType` of `BSplineInterpolationWeightFunction`. This is the type of a table, used internally by its `Evaluate` member function. Declared this table `const`, and initialized the table by an in-class default-member-initializer. Defaulted the default-constructor. Follow-up to pull request InsightSoftwareConsortium#2712 commit bc7c5df "PERF: Use FixedArray for BSplineBaseTransform ParameterIndexArrayType" commit 9bf745b "PERF: Use FixedArray for BSplineInterpolationWeightFunction OutputType"

Replaced `itk::Array2D` by `itk::FixedArray`, used as private `TableType` of `BSplineInterpolationWeightFunction`. This is the type of a table, used internally by its `Evaluate` member function. Declared this table `const`, and initialized the table by an in-class default-member-initializer. Defaulted the default-constructor. Follow-up to pull request #2712 commit bc7c5df "PERF: Use FixedArray for BSplineBaseTransform ParameterIndexArrayType" commit 9bf745b "PERF: Use FixedArray for BSplineInterpolationWeightFunction OutputType"

Code like `WeightsType weights(n)` does not necessarily construct an object of `n` weights, as it did before ITK 5.3. It now fills a fixed array of weights by value `n`. This behavior change is caused by a change of the definition of `itk::BSplineInterpolationWeightFunction::WeightsType`, by ITK pull request InsightSoftwareConsortium/ITK#2712 commit InsightSoftwareConsortium/ITK@9bf745b "PERF: Use FixedArray for BSplineInterpolationWeightFunction OutputType" (merged on 7 September 2021, included with ITK v5.3rc01). Follow-up to pull request #620 commit a0cf523 "COMP: Upgrade ITK from v5.2.0 to v5.3rc03" (merged on 18 February 2022).

dzenanz reviewed Sep 3, 2021

View reviewed changes

Modules/Registration/Common/include/itkImageToImageMetric.hxx Outdated Show resolved Hide resolved

N-Dekker force-pushed the BSplineInterpolationWeightFunction-FixedArray branch from 41fecd6 to 7c79455 Compare September 4, 2021 08:57

N-Dekker force-pushed the BSplineInterpolationWeightFunction-FixedArray branch from e83f1ef to a20047a Compare September 4, 2021 22:02

github-actions bot added the area:Python wrapping Python bindings for a class label Sep 4, 2021

N-Dekker mentioned this pull request Sep 6, 2021

Difference between ITK and elastix BSplineTransform dealing with input points at the border SuperElastix/elastix#509

Open

N-Dekker force-pushed the BSplineInterpolationWeightFunction-FixedArray branch from a20047a to a5cf690 Compare September 6, 2021 14:37

N-Dekker added 2 commits September 6, 2021 21:28

N-Dekker force-pushed the BSplineInterpolationWeightFunction-FixedArray branch from a5cf690 to bc7c5df Compare September 6, 2021 19:29

dzenanz reviewed Sep 6, 2021

View reviewed changes

Wrapping/WrapITKTypes.cmake Show resolved Hide resolved

N-Dekker marked this pull request as ready for review September 7, 2021 09:47

dzenanz approved these changes Sep 7, 2021

View reviewed changes

dzenanz requested a review from blowekamp September 7, 2021 15:14

thewtex approved these changes Sep 7, 2021

View reviewed changes

thewtex merged commit 63c8594 into InsightSoftwareConsortium:master Sep 7, 2021

N-Dekker mentioned this pull request Sep 8, 2021

PERF: Use FixedArray for table within BSplineInterpolationWeightFunction #2721

Merged

N-Dekker mentioned this pull request Sep 10, 2021

BUG: Improve dimension-dependent BSplineInterpolationWeightFunction wrapping #2729

Merged

N-Dekker mentioned this pull request Feb 22, 2022

COMP: Upgrade ITK from v5.2.0 to v5.3rc03 SuperElastix/elastix#620

Merged

N-Dekker mentioned this pull request Feb 22, 2022

STYLE: Do not use WeightsType(n) constructor (related to ITK 5.3) SuperElastix/elastix#622

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

PERF: Use FixedArray for BSplineInterpolationWeightFunction OutputType#2712

PERF: Use FixedArray for BSplineInterpolationWeightFunction OutputType#2712
thewtex merged 2 commits intoInsightSoftwareConsortium:masterfrom
N-Dekker:BSplineInterpolationWeightFunction-FixedArray

N-Dekker commented Sep 3, 2021 •

edited

Loading

Uh oh!

N-Dekker commented Sep 3, 2021

Uh oh!

dzenanz left a comment

Uh oh!

Uh oh!

N-Dekker commented Sep 3, 2021

Uh oh!

dzenanz commented Sep 3, 2021

Uh oh!

thewtex commented Sep 4, 2021

Uh oh!

N-Dekker commented Sep 4, 2021

Uh oh!

dzenanz commented Sep 4, 2021

Uh oh!

N-Dekker commented Sep 4, 2021

Uh oh!

Uh oh!

N-Dekker commented Sep 7, 2021

Uh oh!

thewtex left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

N-Dekker commented Sep 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

N-Dekker commented Sep 3, 2021

Uh oh!

dzenanz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

N-Dekker commented Sep 3, 2021

Uh oh!

dzenanz commented Sep 3, 2021

Uh oh!

thewtex commented Sep 4, 2021

Uh oh!

N-Dekker commented Sep 4, 2021

Uh oh!

dzenanz commented Sep 4, 2021

Uh oh!

N-Dekker commented Sep 4, 2021

Uh oh!

Uh oh!

N-Dekker commented Sep 7, 2021

Uh oh!

thewtex left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

N-Dekker commented Sep 3, 2021 •

edited

Loading