Skip to content

Promote small-integer inputs before computing regression metrics - #9247

Open
leecoring wants to merge 2 commits into
Project-MONAI:devfrom
leecoring:9246-small-integer-regression-metrics
Open

leecoring wants to merge 2 commits into
Project-MONAI:devfrom
leecoring:9246-small-integer-regression-metrics

Conversation

@leecoring

Copy link
Copy Markdown

Fixes #9246.

Description

This change fixes incorrect subtraction for pairs of uint8, int8, and int16 images in MONAI regression metrics. Previously, subtraction could overflow or underflow in the integer dtype; converting the resulting error to floating point afterward could not recover the correct difference. MAE could also fail when taking the mean of an integer tensor. For uint8 images containing 255 and 0, MSE is 1 instead of 65025, RMSE is 1 instead of 255, and PSNR is approximately 48.13 dB instead of 0 dB.

When both inputs use these small-integer dtypes, convert them at the shared RegressionMetric entry point before computation, using the default floating-point dtype with a float32 floor. Individual metric formulas are unchanged. Existing floating-point and mixed floating/integer paths are left unchanged, and the precision tests retain float64 residuals. Wider integer inputs are deliberately outside this change because casting large int32/int64 values can erase unit differences. Direct functional helper calls are outside this patch.

Add regression coverage for five error metrics, all three input dtypes, both subtraction directions, tensor batches and image lists, aggregation, float64 precision, and wider-integer residual preservation. The shared entry point also feeds SSIM and MS-SSIM: comparisons with equivalent float32 images cover both 2D and 3D, including the previous integer-pooling failure in MS-SSIM. This does not change MS-SSIM size validation or its pooling schedule.

Validation

Local CPU checks run by Codex:

  • New test class: before the fix, 24 failures and 8 errors out of 48 tests; after the fix, all 48 pass.
  • python -m unittest tests.metrics.test_compute_regression_metrics tests.metrics.test_ssim_metric tests.metrics.test_compute_multiscalessim_metric -v: 62 tests pass, none skipped, including the 48 new tests.
  • Changed-file pre-commit checks and ./runtests.sh -f: pass (copyright, isort, Black, Ruff, and Pyrefly, using the repository's configured exclusions/suppressions).
  • Synthetic 2×3×4 uint8 NIfTI images saved and reloaded with their dtype preserved: all ten metric comparisons agree with independent NumPy float64 reference calculations within rtol=1e-5, atol=1e-4. This is an additional local diagnostic; binary image files are not added to the PR.
  • Full MONAI unit/integration suites, GPU/distributed tests, other platform/version combinations, and the documentation build were not run. GitHub CI results are not yet available.

Environment: macOS CPU, Python 3.14.7, PyTorch 2.15.0, NumPy 2.5.4, NiBabel 5.4.2 for the additional file diagnostic only. No new runtime or test dependency is introduced.

Verified against upstream dev commit 827fa24fef4f474f70ead141412a2a0675990684. The same new tests were run against a separate unmodified checkout of this commit and against the patched checkout; the results above include this revalidation.

Types of changes

  • Non-breaking change (bug fix; no API signature changes).
  • New tests added to cover the changes.
  • In-line docstrings updated.
  • Integration tests passed locally by running ./runtests.sh -f -u --net --coverage.
  • Quick tests passed locally by running ./runtests.sh --quick --unittests --disttests.
  • Documentation updated, tested make html command in the docs/ folder.

Tool assistance

The submitter provided the explanation of the problem and fix. OpenAI Codex assisted with the investigation, implementation, English translation and preparation of this description, and test execution. The local checks listed above were run by Codex. The validation results describe the tested cases, rather than a guarantee for all input combinations.

Convert pairs of uint8, int8, and int16 inputs before metric computation
to prevent integer subtraction overflow and unsupported reductions.
Preserve existing floating-point and wider-integer calculation paths.

Add regression cases for metric values, batch and list inputs,
aggregation, precision preservation, and image similarity metrics.

Assisted-by: OpenAI Codex
Signed-off-by: leecoring <109904107+leecoring@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 613519a and c2116b0.

Files selected for processing: 1
  • monai/metrics/regression.py
Files skipped from review as they are similar to previous changes: 1
  • monai/metrics/regression.py

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

Walkthrough

When both inputs use uint8, int8, or int16, RegressionMetric converts them to a floating-point dtype before metric computation. Tests cover regression metric values, input and result dtypes, large integer residuals, and SSIM and MS-SSIM comparisons.

Estimated code review effort: 2 (Simple) | ~10 minutes

Fixed issue severity: <fixed_issue_severity>Medium</fixed_issue_severity>

Merge Risk: Minimal · up to c2116

No actionable merge-blocking issue is identified; the change is ready for normal checks.

Pre-merge checks: Passed 5
Check name Status Explanation
Title check Passed The title clearly and concisely describes the primary change: promoting small-integer inputs before regression metric computation.
Description check Passed The description includes the issue reference, change summary, scope, tests, validation results, limitations, environment, and change types. It is complete and aligned with the repository template.
Linked Issues check Passed Issue #9246 requires promotion of paired uint8, int8, and int16 inputs before subtraction. The PR summary and change summary state that RegressionMetric converts both inputs to a floating-poin…
Out of Scope Changes check Passed The reported source change is limited to RegressionMetric input handling and documentation. The reported tests cover the promoted metrics and the shared SSIM and MS-SSIM entry point. The summaries s…
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Full details: Linked Issues check

Explanation

Issue #9246 requires promotion of paired uint8, int8, and int16 inputs before subtraction. The PR summary and change summary state that RegressionMetric converts both inputs to a floating-point dtype with at least float32 precision. Floating-point and mixed-dtype paths remain unchanged. The summaries also report tests for the five affected metrics, both subtraction directions, batch and list inputs, aggregation, float64 precision, and wider-integer residuals. This covers the stated coding requirements.

Full details: Out of Scope Changes check

Explanation

The reported source change is limited to RegressionMetric input handling and documentation. The reported tests cover the promoted metrics and the shared SSIM and MS-SSIM entry point. The summaries state that metric formulas, direct functional helpers, mixed-dtype paths, and wider-integer behavior remain outside the change. These changes support issue #9246 and do not show unrelated scope.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 4 functions across 2 files.

Finishing Touches
Generate unit tests (beta)
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

Document RegressionMetric._compute_tensor input validation, small-integer promotion, arguments, and return values. This is a docstring-only follow-up; metric computations and tests are unchanged.

Local validation run by OpenAI Codex: 62 focused tests pass; changed-file pre-commit checks pass.

Assisted-by: OpenAI Codex

Signed-off-by: leecoring <109904107+leecoring@users.noreply.github.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Small-integer inputs produce incorrect regression metric values or runtime errors

1 participant