Skip to content

BUG: Expose displacement field transform adaptors in Python - #6998

Open
sdiebolt wants to merge 1 commit into
InsightSoftwareConsortium:mainfrom
sdiebolt:fix/6994-syn-python-transform-adaptors
Open

sdiebolt wants to merge 1 commit into
InsightSoftwareConsortium:mainfrom
sdiebolt:fix/6994-syn-python-transform-adaptors

Conversation

@sdiebolt

Copy link
Copy Markdown

Hi! This exposes the existing displacement-field adaptors in Python so multi-resolution SyN can be configured as in the C++ test. Closes #6994.

Scope and local testing

The C++ API and SyN defaults are unchanged: callers still supply per-level adaptors and initialized displacement fields. Automatic field adaptation is left for a follow-up.

  • Wrapped the adaptor hierarchy and its smart-pointer vector.
  • Added 2D and 3D regression tests for multi-resolution SyN, including forward/inverse field geometry and adaptor-count validation.
  • Built the affected Python bindings on Linux; all seven targeted registration-method Python tests passed.
  • Checked float/double adaptor inheritance and container conversion in 2D and 3D.
  • Confirmed that the original example still reproduces the physical-space error without adaptors.
  • All local commit hooks passed. Full-suite and cross-platform validation remain pending.
PR Checklist

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)
  • 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)

Only additive Python bindings are introduced; the existing C++ API and its documentation are unchanged. No new major feature requires a separate examples contribution. The Python test has the standard license header; the wrapping file follows neighboring .wrap files.

AI assistance

AI assistance contributed to diagnosis, wrapping changes, and regression tests. The diff was human-reviewed and the focused checks passed locally.

Multi-resolution SyN requires per-level field adaptation, but Python
cannot configure the necessary adaptors.

Closes InsightSoftwareConsortium#6994
@github-actions github-actions Bot added type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots area:Python wrapping Python bindings for a class type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct area:Registration Issues affecting the Registration module labels Oct 23, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for contributing a pull request! 🙏

Welcome to the ITK community! 🤗👋☀️

We are glad you are here and appreciate your contribution. Please keep in mind our community participation guidelines. 📜
More support and guidance on the contribution process can be found in our contributing guide. 📖

This is an automatic message. Allow for time for the ITK community to be able to read the pull request and comment
on it.

@sdiebolt

sdiebolt commented Oct 23, 2026 •

Copy link
Copy Markdown
Author

Also quick question: would automatic per-level field adaptation be a useful follow-up for SyN? It could make the common multi-resolution setup easier, while keeping explicit adaptors available for customization.

@sdiebolt
sdiebolt marked this pull request as ready for review October 23, 2026 22:13
@greptile-apps

greptile-apps Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; no actionable issues were found.

Summary

Exposes displacement-field transform adaptors and their smart-pointer vector in Python.

  • Python SyN callers can supply displacement-field adaptors for each level.

Reviews (1) · Last reviewed commit: "BUG: Expose displacement field transform..." · Reviewed by Greptile

@hjmjohnson hjmjohnson left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding these bindings; wrapping the adaptors is the right way to make multi-resolution SyN usable from Python. A few changes before this can go in:

  1. Use ENH: rather than BUG: for the commit. It adds new Python API; nothing that existed was broken.
  2. Drop the float DisplacementFieldTransform adaptor templates, or point to a wrapped registration method that uses them. Every wrapped SyN variant uses a double transform, so the float templates add wrapping with no consumer.
  3. Make the final check able to fail. Fixed and moving are the same image, so assert_allclose(field, 0) only shows that nothing moved, and the last level runs at full resolution, so the size, origin, and spacing checks pass whether or not the adaptors resampled. A moving image shifted by a known amount, with a check that the field moves toward it, would test the registration result.
  4. One class per .wrap file: move itk::TransformParametersAdaptor to its own itkTransformParametersAdaptor.wrap.
  5. Use the same dimension variable in both .wrap files. itkTransformParametersAdaptorBase.wrap uses ITK_WRAP_DIMS and the new file uses ITK_WRAP_IMAGE_DIMS; they are aliases, so pick one for consistency.
Smaller changes, also requested
  1. The DFT template foreach is written twice in the new file and repeats the list in itkDisplacementFieldTransform.wrap; define the list once and reuse it.
  2. In wrapping/test/CMakeLists.txt, follow the neighboring guard idiom (list(FIND ITK_WRAP_IMAGE_DIMS ...) / GREATER -1, as in ImageGradient/wrapping/test/CMakeLists.txt) instead of IN_LIST.
  3. Name the literals in the test (16, [1, 0], atol=1e-6), and drop the random image: once fixed and moving differ, a simple deterministic image (for example, a smooth blob) is enough.
Possible follow-ups (not needed in this PR)
  • BSplineSmoothingOnUpdateDisplacementFieldTransformParametersAdaptor is still unwrapped, although SyN is wrapped with that output transform and it is the default for BSplineSyNImageRegistrationMethod.
  • The test builds the container with itk.vector[...] and push_back. Whether a plain Python list converts for SetTransformParametersAdaptorsPerLevel, which takes the container by non-const reference, is untested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:Python wrapping Python bindings for a class area:Registration Issues affecting the Registration module type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Multi-resolution SyN fails in Python when changing shrink factors between levels

2 participants