Skip to content

Wrap BSplineTransform for 2D-2D and 4D-4D case - #6856

Merged
hjmjohnson merged 1 commit into
InsightSoftwareConsortium:release-5.4from
dzenanz:bspline2D2D
Sep 11, 2026
Merged

Wrap BSplineTransform for 2D-2D and 4D-4D case#6856
hjmjohnson merged 1 commit into
InsightSoftwareConsortium:release-5.4from
dzenanz:bspline2D2D

Conversation

@dzenanz

@dzenanz dzenanz commented Sep 10, 2026

Copy link
Copy Markdown
Member

This is a backport of #6396 to 5.4 release branch.

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)
  • Added test (or behavior not changed)
  • Added Python wrapping to new files (if any) as described in ITK Software Guide Section 9.5

Currently available combinations are as follows:

<itkTemplate itk::BSplineTransform>
Options:
  [<itkCType double>, 2, 3]
  [<itkCType double>, 3, 3]
  [<itkCType double>, 4, 3]
@github-actions github-actions Bot added type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots area:Python wrapping Python bindings for a class area:Core Issues affecting the Core module labels Sep 10, 2026
@dzenanz
dzenanz marked this pull request as ready for review September 10, 2026 20:43
@greptile-apps

greptile-apps Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This change adds dimension-equals-spline-order BSpline Python wrappers and instantiation tests. Two wrapping failures remain: the new 2D test is registered even when 2D wrapping is disabled, and the new 4D order-4 transform exposes APIs requiring 625-element FixedArrays that are not generated. The introduced variable names also do not meet the repository naming requirement.

Confidence Score: 3/5

Not safe to merge until the two Python wrapping failures are fixed and the repository naming requirement is satisfied.

Two independently reproduced failures leave valid reduced wrapping configurations with a failing test and leave the new 4D order-4 Python API incomplete.

Files Needing Attention: Modules/Core/Transform/wrapping/test/CMakeLists.txt, Modules/Core/Transform/wrapping/itkBSplineTransform.wrap, and Wrapping/WrapITKTypes.cmake

T-Rex T-Rex Logs

What T-Rex did

  • T-Rex produced proofs for multiple posted P1 findings and linked them to the corresponding review comments.
  • T-Rex produced a proof for another P1 finding, but no artifacts were provided.
  • T-Rex performed a general contract validation for bspline-2d2d-condition, capturing the before-state results and the after-state result using the authored script.
  • T-Rex produced a P1 finding proof for a different case, with its supporting artifact available for review.
  • T-Rex performed a separate bspline4d_fixedarray_signature_validation, recording the before/after captures and the executable-path details.

View all artifacts

T-Rex Ran code and verified through T-Rex

Comments Outside Diff (2)

  1. General comment

    P1 Guard the 2D-2D Python expression test by configured wrapping dimensions

    • Bug
      • With ITK_WRAP_IMAGE_DIMS=3, candidate 0 still registers PythonInstantiateBSplineTransform2D2D, but its required BSplineTransform<double,2,2> specialization is absent. The focused candidate validation exited 1 with this mismatch.
    • Cause
      • The added 2D-2D test has no list(FIND ITK_WRAP_IMAGE_DIMS 2 ...) guard, unlike the newly added 4D-4D test. The wrapper file emits the 2D-2D specialization only within foreach(d ${ITK_WRAP_IMAGE_DIMS}) when d equals 2.
    • Fix
      • Wrap PythonInstantiateBSplineTransform2D2D in a check that ITK_WRAP_IMAGE_DIMS contains 2, matching the conditional registration used for the 4D-4D test.

    T-Rex Ran code and verified through T-Rex

  2. General comment

    P1 4D order-4 BSplineTransform wrapper has ungenerated FixedArray dependencies

    • Bug
      • The added BSplineTransform<double,4,4> wrapper inherits public TransformPoint and Jacobian-related signatures with WeightsType and ParameterIndexArrayType. For spline order 4 in dimension 4, NumberOfWeights is (4 + 1)^4 = 625, so those types are FixedArray<double,625> and FixedArray<unsigned long,625>. The changed FixedArray configuration retains the order-3 4^d computation and produces 16, 64, and 256 for dimensions 2, 3, and 4; neither 625 type is generated. Generated Python signatures therefore become unavailable or incomplete.
    • Cause
      • Wrapping/WrapITKTypes.cmake calculates BSpline FixedArray sizes with a hard-coded multiplication by 4, which matches spline order 3 but not the newly exposed order-4 template.
    • Fix
      • Generate the FixedArray specializations required by every exposed BSpline order, specifically add double and unsigned-long size 625 when wrapping BSplineTransform<double,4,4> (preferably derive the size from VSplineOrder + 1 rather than hard-coding 4).

    T-Rex Ran code and verified through T-Rex

Reviews (1): Last reviewed commit: "ENH: Wrap BSplineTransform for 2D-2D and..." | Re-trigger Greptile

Comment thread Modules/Core/Transform/wrapping/test/CMakeLists.txt
Comment thread Modules/Core/Transform/wrapping/itkBSplineTransform.wrap
Comment thread Modules/Core/Transform/wrapping/test/CMakeLists.txt
@dzenanz

dzenanz commented Sep 10, 2026

Copy link
Copy Markdown
Member Author

/azp run ITK.macOS.Python

@dzenanz dzenanz mentioned this pull request Sep 11, 2026
@hjmjohnson
hjmjohnson merged commit d173411 into InsightSoftwareConsortium:release-5.4 Sep 11, 2026
17 checks passed
@dzenanz
dzenanz deleted the bspline2D2D branch September 11, 2026 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:Core Issues affecting the Core module area:Python wrapping Python bindings for a class type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants