Skip to content

BUG: Smooth structure tensor with an isotropic Gaussian kernel#6602

Merged
hjmjohnson merged 1 commit into
InsightSoftwareConsortium:mainfrom
physwkim:bug-structure-tensor-single-axis-smoothing-6575
Jul 16, 2026
Merged

BUG: Smooth structure tensor with an isotropic Gaussian kernel#6602
hjmjohnson merged 1 commit into
InsightSoftwareConsortium:mainfrom
physwkim:bug-structure-tensor-single-axis-smoothing-6575

Conversation

@physwkim

@physwkim physwkim commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

StructureTensorImageFilter smoothed the tensor field with RecursiveGaussianImageFilter, which filters one axis only. The structure tensor requires an isotropic kernel, so both scales now use SmoothingRecursiveGaussianImageFilter. Addresses B22 of #6575.

The output change is intentional and maintainer-confirmed; all 16 AnisotropicDiffusionLBR baselines are regenerated in this PR (blobs merged via InsightSoftwareConsortium/ITKTestingData#77) and the ITK 6 migration guide documents the change for downstream consumers. Only SimpleITK is expected to need new baselines.

Root cause

itkStructureTensorImageFilter.hxx:124 built the feature-scale (K_rho) smoother as

using GaussianFilterType = RecursiveGaussianImageFilter<TensorImageType>;

RecursiveGaussianImageFilter derives from RecursiveSeparableImageFilter and filters along a single direction — m_Direction, which defaults to 0 and was never set here. The structure tensor K_rho * (grad(u_sigma) (x) grad(u_sigma)) requires an isotropic kernel, i.e. smoothing on every axis. The same mistake appears at :75 for the noise-scale (K_sigma) smoother, on the m_UseGradientRecursiveGaussianImageFilter == false branch (unreachable — the flag has no setter — but the same defect, so it is fixed too). Both now use SmoothingRecursiveGaussianImageFilter, which chains one RecursiveGaussianImageFilter per axis. GradientRecursiveGaussianImageFilter, used elsewhere in the same file, already loops SetDirection() over every axis and was correct.

The single-axis smoothing dates to the original 2014 implementation (Jean-Marie Mirebeau) and was unintentional.

Baselines and migration

All 16 regenerated: PacMan_cEED / cCED / I, FingerPrint_cEED / cCED / I, Cos3D_cCED, mrbrain_cEED, Lena_Detail_cCED_2 / cEED_2 / I_2, VectorField_Circle_cEED, Triangle_cEED / Triangle_EED, Oscillations1_cCED / Oscillations1_CED.

Generated on macOS arm64 from the fixed filter; CIDs computed with npx ipfs-car pack --no-wrap (unixfs-v1-2025 profile). .cid content links and content-links.manifest are updated in this commit. itk_6_migration_guide.md explains why the structure tensor requires an isotropic kernel and what changes for CoherenceEnhancingDiffusionImageFilter / AnisotropicDiffusionLBRImageFilter.

Regression test

itkStructureTensorImageFilterGTest.cxx gains StructureTensorImageFilter.FeatureScaleSmoothsAlongAllAxes, which loops a step edge over both axes of a symmetric 64x64 fixture and asserts a feature-scale trace delta on each axis independently. Verified to fail against the pre-fix single-axis source (stepAxis=1 delta = 0), so it is a real tripwire. ITKSmoothing added to the module's DEPENDS for the new include.

AI assistance

@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 labels Jul 13, 2026
@hjmjohnson

Copy link
Copy Markdown
Member

Status of this draft: blocked on baselines and now on a rebase conflict, and a high-effort automated review found two confirmed behavioral regressions in the smoothing swap.

Blockers

  • The Pixi-Cxx failures on all three platforms are the CoherenceEnhancing/EdgeEnhancing example tests (PacMan_*, FingerPrint_*, Cos3D_*, Lena_*) failing against their committed baselines — the behavior change requires regenerated baselines (or tolerances) before CI can pass.
  • The branch no longer rebases cleanly onto main: a same-named itkStructureTensorImageFilterGTest.cxx was added there, so the test needs merging with the upstream one.
Review findings (confirmed)
  1. SmoothingRecursiveGaussianImageFilter throws for images with <4 pixels along any axis, where the old axis-0-only recursion tolerated thin images — previously-working pipelines on thin/single-slice inputs now hard-fail.
  2. The swap silently smooths through NumericTraits<PixelType>::FloatType (float for the default double tensor components) — a precision downgrade unrelated to the axis bug.
  3. The Dispatch<false> fix site is dead code: m_UseGradientRecursiveGaussianImageFilter defaults to true with no setter, so that branch is unreachable and unexercised by the new GTest.
  4. Minor: the heavier gaussianFilter is now constructed unconditionally per component but only used in the unreachable branch; the test's EXPECT_GT(delta, 1.0) floor is ~3 orders of magnitude below the true delta and would miss magnitude errors.

@hjmjohnson
hjmjohnson force-pushed the bug-structure-tensor-single-axis-smoothing-6575 branch 2 times, most recently from 1cceb81 to 0008d4c Compare July 16, 2026 12:26
@github-actions github-actions Bot removed the type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots label Jul 16, 2026
@hjmjohnson

Copy link
Copy Markdown
Member

Rebased onto main and strengthened the regression test. Two force-pushes, kept separate so the compare links stay readable:

  1. 850ebde…1cceb81 — pure rebase onto current main (173 commits). One real collision: main had independently added itkStructureTensorImageFilterGTest.cxx (the sibling B23 zero-max-trace fix), so the two test suites are merged into one file sharing an anonymous-namespace ImageType/FilterType.
  2. 1cceb81…0008d4c — test only: FeatureScaleSmoothsAlongAllAxes now loops the step edge over both axes on a symmetric 64×64 fixture.
Why the test changed

The prior fixture was x-constant (8×64, y-step only), so isotropic x-smoothing was a no-op on it — the assertion proved some cross-axis smoothing but couldn't distinguish "all axes" from "y only", and would not catch a regression that dropped primary-axis smoothing while keeping y. The two-axis loop asserts a feature-scale trace delta on each axis independently.

Verified locally (AnisotropicDiffusionLBRGTestDriver):

  • With the fix: both axes pass.
  • Against the pre-fix single-axis source: stepAxis=1 fails with delta = 0 — a real tripwire.

pre-commit run --all-files clean; single commit preserved.

The 16 AnisotropicDiffusionLBR image-comparison baselines remain invalidated by the isotropic-smoothing fix — still the open maintainer question (regenerate + ExternalData upload). PR stays draft pending that decision.

@hjmjohnson
hjmjohnson force-pushed the bug-structure-tensor-single-axis-smoothing-6575 branch from 0008d4c to 40d9e20 Compare July 16, 2026 12:45
@github-actions github-actions Bot added type:Data Changes to testing data area:Documentation Issues affecting the Documentation module labels Jul 16, 2026
@hjmjohnson

Copy link
Copy Markdown
Member

Maintainer confirmed this is a genuine bug. Pushed the follow-through — the PR is now complete on the code side and awaits its baseline data:

  • Regenerated all 16 CED/EED/LBR baselines for the corrected isotropic output; .cid content links + content-links.manifest updated in this commit. The new blobs are in DATA: Add isotropic-smoothing AnisotropicDiffusionLBR baselines (ITK #6602) ITKTestingData#77 (draft).
  • Migration guide entry added (itk_6_migration_guide.md) explaining why the structure tensor requires an isotropic kernel and what changes downstream for CoherenceEnhancingDiffusionImageFilter / AnisotropicDiffusionLBRImageFilter.

CI ordering: the Pixi-Cxx image-comparison jobs will stay red until ITKTestingData#77 merges and the GitHub Pages mirror serves the new blobs. Merge the data PR first, then re-run CI here.

Force-push history (all --force-with-lease)
  1. 850ebde…1cceb81 — pure rebase onto main (173 commits; merged the add/add GTest collision with the sibling B23 zero-max-trace fix)
  2. 1cceb81…0008d4c — test strengthening (two-axis FeatureScaleSmoothsAlongAllAxes)
  3. 0008d4c…40d9e20this push: 16 regenerated baselines + migration guide

Base unchanged across pushes 2–3, so each compare link shows only its own delta.

Baseline provenance

Generated on macOS arm64 from the fixed filter, CIDs computed with npx ipfs-car pack --no-wrap (unixfs-v1-2025 profile). The strengthened GTest was verified to fail against the pre-fix single-axis source (stepAxis=1 delta = 0), so it is a real regression tripwire. If cross-platform CI reveals tolerance-exceeding diffs on any of the 16, supplemental per-platform baselines may be needed.

@hjmjohnson
hjmjohnson force-pushed the bug-structure-tensor-single-axis-smoothing-6575 branch from 40d9e20 to de548a8 Compare July 16, 2026 13:07
@hjmjohnson
hjmjohnson marked this pull request as ready for review July 16, 2026 16:55
@hjmjohnson

Copy link
Copy Markdown
Member

It appears that only SimpleITK will need new baselines.

@greptile-apps

This comment was marked as low quality.

RecursiveGaussianImageFilter smooths along a single axis only
(direction 0 by default). The structure tensor definition
K_rho(grad(u_sigma) (x) grad(u_sigma)) requires an isotropic
Gaussian kernel at both scales, smoothing every axis. Use
SmoothingRecursiveGaussianImageFilter, which smooths all axes.

All 16 CoherenceEnhancingDiffusion/AnisotropicDiffusionLBR baselines
are regenerated for the corrected isotropic output, and the migration
guide documents the behavior change for downstream consumers.

Addresses item B22 of InsightSoftwareConsortium#6575.
@hjmjohnson
hjmjohnson force-pushed the bug-structure-tensor-single-axis-smoothing-6575 branch from de548a8 to 0b55b82 Compare July 16, 2026 19:17
@hjmjohnson

Copy link
Copy Markdown
Member

Force-pushed de548a8…0b55b82pure rebase onto current main (15 commits), no content changes. This clears the CONFLICTING state; the PR is now MERGEABLE.

Conflict resolution

One conflict, in Documentation/docs/migration_guides/itk_6_migration_guide.md — an add/add at the end of the file. main appended the projection collapsed-axis (#6594) and HDF5 compression (#6649) sections where this PR appended its structure-tensor section. Resolved by keeping all three, in that order.

Testing/Data/content-links.manifest auto-merged: this PR's delta touches only the 16 AnisotropicDiffusionLBR baselines, and the HeadMRVolumeMaximumProjection* entries added by #6594 are preserved. Verified every .cid file still matches its manifest row.

git range-diff de548a8~1..de548a8 0b55b82~1..0b55b82 shows the patch itself is byte-identical — only the migration-guide append anchor moved. pre-commit run --all-files clean.

@hjmjohnson
hjmjohnson merged commit 0acef9a into InsightSoftwareConsortium:main Jul 16, 2026
17 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:Documentation Issues affecting the Documentation module area:Filtering Issues affecting the Filtering module type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances type:Data Changes to testing data 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.

2 participants