BUG: Fix POISSON step-size collapse and Riemannian max seeds in PatchBasedDenoising

[physwkim]

static_cast<PixelValueType>(0.99999) truncates to 0 for any integer pixel type, collapsing the POISSON fidelity step size to 1e-5 for every integer-pixel image; and two running-maximum accumulators are seeded with NumericTraits<T>::min(), the smallest positive float. Addresses B26 of #6575.

Root cause

POISSON step cap: the cap is compared against the output value after being cast to the pixel storage type. POISSON constrains pixel values to be non-negative, so for an integer type std::min(outVal, 0) is always 0 and the step size is a constant regardless of the actual value — the fidelity term is silently defeated. The cap is now applied in the real-valued domain.

Riemannian max seeds: maxNorm and m_ImageMax track a running maximum but start at NumericTraits<T>::min(), which for floating-point T is the smallest positive normal, not the range minimum. An all-zero or all-negative norm never beats the seed, so the tracked maximum sticks at the epsilon. NonpositiveMin() is the correct seed.

Local validation

New GoogleTest itkPatchBasedDenoisingImageFilterGTest.cxx in a new driver.

• PoissonFidelityAffectsIntegerPixelOutput — integer-pixel image, POISSON, comparing NoiseModelFidelityWeight 0 against 1. Fail-before: the two outputs barely differ (sumAbsDiff = 98), because the fidelity term is inert. Pass-after: sumAbsDiff = 1152.
• ConstantTensorImageRejectedAsNonconstant — an all-identity-tensor image driven through the multithreaded Riemannian min/max pipeline. Fail-before: the filter runs a full denoising iteration on a genuinely constant image, because the epsilon seed keeps max != min. Pass-after: the pre-existing "each image component must be nonconstant" guard fires as intended.

ctest -R Denoising: no baseline flipped. The legacy Poisson baseline uses float pixels and the legacy tensor baseline is not the degenerate all-identity case, so neither was exposed.

AI assistance

• Tool: Claude Code
• Role: implementation and regression-test authoring from the analysis in BUG: Findings collected while porting ~60 filters — reported together in one issue #6575
• All code was reviewed, built, and tested locally before committing

[greptile-apps]

Greptile Summary

This PR fixes PatchBasedDenoising numeric edge cases and adds regression coverage.

• Computes the POISSON fidelity step cap in the real-valued domain.
• Seeds Riemannian running maxima with NonpositiveMin().
• Adds GoogleTest coverage for integer POISSON fidelity and constant tensor images.
• Wires the Denoising tests to ITKGoogleTest.

Confidence Score: 5/5

This looks safe to merge.

No blocking issues found in the changed code.

No files need attention.

T-Rex Logs

What T-Rex did

• T-Rex observed that the denoising tooling environment lacked cmake and ninja, while compilers and make were present.
• An explicit narrow configure attempt failed with /bin/sh: 1: cmake: not found (exit code 127), confirming cmake availability blocks the build.
• Registration checks showed both named tests exist and CMake/module registration checks completed with exit code 0.
• A registration harness script was generated and used to produce the verification proof.

_{Ran code and verified through T-Rex}

Important Files Changed

Filename	Overview
Modules/Filtering/Denoising/include/itkPatchBasedDenoisingImageFilter.hxx	Updates the POISSON step-size calculation and Riemannian max seeds with type-consistent numeric traits usage.
Modules/Filtering/Denoising/itk-module.cmake	Adds the GoogleTest test dependency needed by the new test driver.
Modules/Filtering/Denoising/test/CMakeLists.txt	Adds the Denoising GoogleTest source list and driver registration.
Modules/Filtering/Denoising/test/itkPatchBasedDenoisingImageFilterGTest.cxx	Adds regression tests for integer POISSON fidelity and constant tensor-image rejection.

_{Reviews (1): Last reviewed commit: "BUG: Seed Riemannian max trackers with N..." | Re-trigger Greptile}

[hjmjohnson]

Force-push adds a follow-up commit fixing integer-domain arithmetic and a NaN path found in review of this PR's own two fixes. ctest -L ITKDenoising: 8/8 pass.

What's fixed

Unsigned wraparound amplified by this PR's own POISSON fix. inVal/outVal were read as PixelValueType in all three noise-model branches, so (inVal - outVal) computed in that narrow type. For an unsigned pixel type of rank ≥ int (e.g. unsigned int), 10u - 90u wraps to 4294967216u, not -80. The pre-existing code masked this: stepSizeFidelity was capped at ~1e-5 via a truncating static_cast<PixelValueType>(0.99999), which for any integer PixelValueType truncates to 0, so the wrapped gradient got multiplied by a near-zero step and barely mattered. This PR's own fix (computing that cap in RealValueType) removes the truncation — correctly — but that also means stepSizeFidelity now reaches its real, much larger value, and multiplies the wrapped gradient by it instead. RICIAN has the same problem one line earlier: inVal * outVal overflows a promoted int for realistic 16-bit pixel values before ever dividing by sigmaSquared. Fixed by reading inVal/outVal as RealValueType in all three branches (GAUSSIAN included, for symmetry — it has the identical subtraction pattern), so every arithmetic step happens in the real domain from the start.

sqrt(negative) → NaN from the Riemannian max-tracker seed. This PR's own fix reseeded maxNorm/m_ImageMax with NonpositiveMin() to fix a real bug (an all-negative or all-zero norm never updating the running max). But foundMinMax is set unconditionally per pixel, and maxNorm only updates via tmpNorm[0] > maxNorm[0] — if every tmpNorm[0] this loop sees is itself NaN (e.g. from a non-positive-definite input tensor), that comparison never fires, maxNorm[0] stays at the seeded value, and std::sqrt(maxNorm[0]) becomes std::sqrt(a negative number) → NaN. Since both quantities are squared geodesic differences (non-negative by construction), 0 is the correct in-domain floor — it fixes the same original bug (a genuinely all-zero norm now correctly resolves to sqrt(0) = 0, not NaN) without introducing a value outside the quantity's natural domain.

Local validation

pre-commit run --all-files: exit 0. ITKDenoisingGTestDriver: 2/2 pass (including this PR's own PoissonFidelityAffectsIntegerPixelOutput, using PixelType=int, confirming the RealValueType change doesn't disturb the intended fix). ctest -L ITKDenoising: 8/8 pass, 0 regressions (Gaussian/Rician/Poisson/Tensors legacy tests included).

I attempted an end-to-end regression test reproducing the NaN with an all-zero tensor background, and confirmed the NaN persists even with this fix — but traced it to a separate, deeper issue: computing the matrix logarithm of a genuinely singular/zero tensor is mathematically undefined (log(0) = -∞) independent of any seed choice, and is out of scope for this fix. Removed that test rather than assert something the fix can't guarantee; the seed change is still a strict, verified improvement for every case it's actually meant to cover.