Repository navigation
Conversation
Multi-resolution SyN requires per-level field adaptation, but Python cannot configure the necessary adaptors. Closes InsightSoftwareConsortium#6994
There was a problem hiding this comment.
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.
|
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. |
[Medium impact] The PR appears safe to merge; no actionable issues were found.
|
There was a problem hiding this comment.
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:
- Use
ENH:rather thanBUG:for the commit. It adds new Python API; nothing that existed was broken. - Drop the float
DisplacementFieldTransformadaptor 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. - 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. - One class per
.wrapfile: moveitk::TransformParametersAdaptorto its ownitkTransformParametersAdaptor.wrap. - Use the same dimension variable in both
.wrapfiles.itkTransformParametersAdaptorBase.wrapusesITK_WRAP_DIMSand the new file usesITK_WRAP_IMAGE_DIMS; they are aliases, so pick one for consistency.
Smaller changes, also requested
- The DFT template
foreachis written twice in the new file and repeats the list initkDisplacementFieldTransform.wrap; define the list once and reuse it. - In
wrapping/test/CMakeLists.txt, follow the neighboring guard idiom (list(FIND ITK_WRAP_IMAGE_DIMS ...)/GREATER -1, as inImageGradient/wrapping/test/CMakeLists.txt) instead ofIN_LIST. - 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)
BSplineSmoothingOnUpdateDisplacementFieldTransformParametersAdaptoris still unwrapped, although SyN is wrapped with that output transform and it is the default forBSplineSyNImageRegistrationMethod.- The test builds the container with
itk.vector[...]andpush_back. Whether a plain Python list converts forSetTransformParametersAdaptorsPerLevel, which takes the container by non-const reference, is untested.
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.
PR Checklist
PR Checklist
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
.wrapfiles.AI assistance
AI assistance contributed to diagnosis, wrapping changes, and regression tests. The diff was human-reviewed and the focused checks passed locally.