Skip to content

BUG: Make TimeVaryingBSplineVelocityField registration work with release flag - #7096

Draft
gdevenyi wants to merge 3 commits into
InsightSoftwareConsortium:mainfrom
gdevenyi:bug-tvbsv-global-release-data-flag
Draft

gdevenyi wants to merge 3 commits into
InsightSoftwareConsortium:mainfrom
gdevenyi:bug-tvbsv-global-release-data-flag

Conversation

@gdevenyi

@gdevenyi gdevenyi commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

Makes TimeVaryingBSplineVelocityFieldImageRegistrationMethod give the same result with DataObject::GlobalReleaseDataFlag on as off. Stacked on #7095; only the last commit (8bf17af) is new.

Root cause

With the flag on, every internal filter releases its inputs after it runs. Several of those inputs are read again later. Turning the flag on in a small 2-D registration and fixing the first failure each time found:

  1. TimeVaryingBSplineVelocityFieldTransformParametersAdaptor feeds the transform's control point lattice to one VectorIndexSelectionCastImageFilter per component. The first selector releases the lattice, and the second throws "region is outside of buffered region".
  2. TimeVaryingBSplineVelocityFieldTransform::IntegrateVelocityField feeds the lattice to BSplineControlPointImageFilter, which releases the transform's parameters.
  3. The registration's fixed and moving resamplers take the virtual domain as a reference image, and they take the level images as inputs. Both get released. The metric then throws "Virtual domain and moving transform displacement field must have the same size".
  4. The forward and inverse integrators share the sampled velocity field. The forward integrator releases it, and the inverse integrator then integrates an empty field. This throws nothing: the inverse displacement field just comes out all zero.
  5. The fixed mask resampler releases the mask image that the metric still uses. The run then segfaults in ImageMaskSpatialObject::IsInsideInObjectSpace.
Fix

Each of those filters now gets a graft, which shares the buffer, so no data is copied and results are unchanged. The registration uses #7095's MakeGraft helper. Its resamplers now set their geometry with SetOutputParametersFromImage(virtualDomainImage), the same change #7095 made in SyN.

Sites left as they are, because nothing reads them again after the release:

  • the inverse integrator's input, since the sampled field is a local that is discarded after the inverse integrator runs;
  • the AddImageFilter input in UpdateTransformParameters, since the old lattice is replaced by the sum on the next line. The registration later reads only its metadata.

The multi-metric resampler branch got the same change as the single-metric branch. The tests run only the single-metric path, as in #7095.

Tests

There are two new cases in #7095's itkSyNImageRegistrationMethodReleaseDataGTest.cxx: a two-level 32x32 time-varying B-spline registration, unmasked and masked. Each compares the flag-on displacement and inverse fields exactly (bitwise) with a flag-off run, and checks that the result is nonzero.

  • With only BUG: Make SyN and velocity field registration work with GlobalReleaseDataFlag #7095 (this commit's three headers reverted), both new cases fail with the lattice exception (site 1).
  • With this commit, all 7 cases in the driver pass. ctest for the ITKRegistrationMethodsv4, ITKDisplacementField and ITKRegistrationCommon labels: 65/65 passed.
  • Mutations. Each graft was removed in turn, and the new cases failed every time:
Removed Result
adaptor selector graft exception: region outside buffered region
B-spline filter lattice graft exception: region outside buffered region
forward integrator graft inverse field differs (0 vs 0.00115)
resampler level-image grafts fields differ from the first value
SetOutputParametersFromImage (back to the reference image) exception: virtual domain size mismatch
mask resampler graft segfault in the masked case
AI assistance

Tool: Claude Code (claude-opus-5-5). Role: found the failing sites by running with the flag on, wrote the fix and tests, and ran the mutation checks. All code was reviewed and tested locally before committing.

@gdevenyi

gdevenyi commented Nov 3, 2026

Copy link
Copy Markdown
Contributor Author

@ntustison could you take a look when you have a moment? This makes TimeVaryingBSplineVelocityFieldImageRegistrationMethod (and its transform and parameters adaptor) give identical results with GlobalReleaseDataFlag on, by grafting the images that internal filters would otherwise release while they are still in use.

@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 type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct area:Filtering Issues affecting the Filtering module area:Registration Issues affecting the Registration module labels Nov 3, 2026
@gdevenyi

gdevenyi commented Nov 3, 2026

Copy link
Copy Markdown
Contributor Author

The Pixi-Cxx (macos-15) failure is unrelated: itkMCI_DSC_case_2_binary in the MorphologicalContourInterpolation remote module segfaulted; that test does not use the time-varying B-spline code this PR changes. Could a maintainer rerun that job?

@gdevenyi
gdevenyi force-pushed the bug-tvbsv-global-release-data-flag branch from 8bf17af to c968cc5 Compare November 3, 2026 13:11
@github-actions github-actions Bot added the area:Core Issues affecting the Core module label Nov 3, 2026
@gdevenyi

gdevenyi commented Nov 3, 2026

Copy link
Copy Markdown
Contributor Author

After the rebase onto the new #7095 (c968cc5), the Azure ITK.Windows and ITK.macOS builds show as failed, but Azure reports both as canceled, with no logs. The GitHub Actions jobs that have finished pass. Could a maintainer rerun the two Azure builds?

@gdevenyi
gdevenyi force-pushed the bug-tvbsv-global-release-data-flag branch 2 times, most recently from 3c00c76 to e64c4a9 Compare November 4, 2026 00:39
@gdevenyi

gdevenyi commented Nov 4, 2026

Copy link
Copy Markdown
Contributor Author

The ARMBUILD-x86_64-rosetta failure is unrelated: ImageToVideoFilterTest could not read its baseline HeadMRVolume_Frame0.mha ("Could not create IO object for reading file"), so the test data was missing on the runner. The same job passes on #7095, which this PR is stacked on. Could a maintainer rerun it?

@gdevenyi

gdevenyi commented Nov 4, 2026

Copy link
Copy Markdown
Contributor Author

@hjmjohnson, could you rerun the failed ARMBUILD-x86_64-rosetta job (https://github.com/InsightSoftwareConsortium/ITK/actions/runs/41748468814/job/126307315016)? I can't rerun jobs myself. ImageToVideoFilterTest could not find its baseline HeadMRVolume_Frame0.mha on the runner, and the same job passes on #7095. Every other check passes.

ImageAdaptor::Graft copied the pixel container but not the pixel
accessor, so a graft of an NthElementImageAdaptor or of a
VectorImageToImageAdaptor read element or component 0, whatever the
source selected. Graft the adapted image, which also carries a
VectorImage's number of components, then copy the accessor. A
GoogleTest reads both kinds of graft at every pixel, before and after
UpdateOutputInformation.
…DataFlag

With DataObject::GlobalReleaseDataFlag on, internal mini-pipelines
released inputs that are read again: the virtual domain (a resampler
reference image), the level images, the masks, the transform fields fed
to the composers, inverters and integrators, and the metric images fed
to the gradient filters. Feed those pipelines grafts, which share the
buffers, and set resampler geometry with SetOutputParametersFromImage.
The velocity field interpolator points back at the velocity field after
integration. Registration results are unchanged. GoogleTests cover each
case with the flag on, including adaptor inputs to the metric.
…ase flag

With DataObject::GlobalReleaseDataFlag on, internal filters released
inputs that are read again: the transform's control point lattice (fed
to the B-spline filter and to the adaptor's component selectors), the
sampled velocity field shared by the forward and inverse integrators,
and the virtual domain, level images and fixed mask fed to the
registration's resamplers. Feed those filters grafts, which share the
buffers, and set resampler geometry with SetOutputParametersFromImage.
Results are unchanged. GoogleTests compare the registration, with and
without masks, with the flag off and on.
@gdevenyi
gdevenyi force-pushed the bug-tvbsv-global-release-data-flag branch from e64c4a9 to 61a2cab Compare November 6, 2026 17:24
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:Filtering Issues affecting the Filtering module 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.

1 participant