Skip to content

BUG: ScaleLogarithmicTransform Jacobian ignores the center#6572

Merged
hjmjohnson merged 1 commit into
InsightSoftwareConsortium:mainfrom
physwkim:fix-scale-log-jacobian
Jul 9, 2026
Merged

BUG: ScaleLogarithmicTransform Jacobian ignores the center#6572
hjmjohnson merged 1 commit into
InsightSoftwareConsortium:mainfrom
physwkim:fix-scale-log-jacobian

Conversation

@physwkim

@physwkim physwkim commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

ScaleLogarithmicTransform's Jacobian dropped the transform center: d/dLog(scale[i]) is scale[i] * (p[i] - center[i]), not scale[i] * p[i]. Adds the missing Jacobian test case. Split out of #6569 per review.

Root cause

itkScaleLogarithmicTransform.hxx computed

jacobian(dim, dim) = scales[dim] * p[dim];

but the transform (inherited from ScaleTransform) maps
p -> scale * (p - center) + center. itkScaleTransform.hxx already
differentiates about the center (j(dim, dim) = p[dim] - center[dim]), so
the log-scale chain rule gives scale[dim] * (p[dim] - center[dim]). With a
non-zero center, any Optimizerv4 driving this transform received a wrong
gradient.

itkScaleLogarithmicTransformTest exercised no Jacobian at all. The added
case: center (5, 6, 7), scales (2, 3, 4), point (10, 10, 10) — expected
diagonal (10, 12, 12); the old code returns (20, 30, 40).

AI assistance

ComputeJacobianWithRespectToParameters() returned scale * p, while the
transform maps p to scale * (p - center) + center. The base
ScaleTransform already differentiates about the center. Registration
with a non-zero center was fed a wrong gradient.

Differentiate about the center, and cover the Jacobian in the test,
which previously exercised none of it.
@github-actions github-actions Bot added type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct area:Core Issues affecting the Core module labels Jul 9, 2026
@physwkim physwkim marked this pull request as ready for review July 9, 2026 14:32
@greptile-apps

greptile-apps Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes the ScaleLogarithmicTransform Jacobian for transforms with a non-zero center. The main changes are:

  • Uses p - center when computing each log-scale Jacobian diagonal entry.
  • Adds a regression test with a non-zero center and non-unit scales.
  • Verifies the expected Jacobian values for the centered transform case.

Confidence Score: 5/5

Safe to merge with minimal risk.

The change is small, matches ScaleTransform::TransformPoint behavior, and is covered by a focused regression test.

No files require special attention.

T-Rex T-Rex Logs

What T-Rex did

  • The initial CTest run for the scale_logarithmic_transform completed successfully, with exit code 0, and its transcript was preserved in the first log.
  • A PIPESTATUS portability issue was observed after the initial run, so the command was rerun under bash to achieve a clean harness exit.
  • The CTest rerun under bash completed successfully, with exit code 0, and its transcript was captured in the rerun log.

View all artifacts

T-Rex Ran code and verified through T-Rex

Important Files Changed

Filename Overview
Modules/Core/Transform/include/itkScaleLogarithmicTransform.hxx Updates the log-scale Jacobian to differentiate around the transform center; no issues found.
Modules/Core/Transform/test/itkScaleLogarithmicTransformTest.cxx Adds a non-zero-center Jacobian regression test using log-scale parameters; no issues found.

Reviews (1): Last reviewed commit: "BUG: ScaleLogarithmicTransform Jacobian ..." | Re-trigger Greptile

@hjmjohnson

Copy link
Copy Markdown
Member

ITK.Linux.Python failed with "We stopped hearing from agent Azure Pipelines 5" (agent VM died mid-build) — infrastructure flake, no code issue; CDash shows 0 errors / 0 test failures for this revision. Retriggering.

@hjmjohnson

Copy link
Copy Markdown
Member

/azp run ITK.Linux.Python

@hjmjohnson hjmjohnson merged commit 812d7a8 into InsightSoftwareConsortium:main Jul 9, 2026
22 checks passed
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 type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances 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