Skip to content

fix(image): correct block_size validation in PSNRB (and → or)#3367

Open
xodn348 wants to merge 1 commit intoLightning-AI:masterfrom
xodn348:fix/psnrb-block-size-validation
Open

fix(image): correct block_size validation in PSNRB (and → or)#3367
xodn348 wants to merge 1 commit intoLightning-AI:masterfrom
xodn348:fix/psnrb-block-size-validation

Conversation

@xodn348
Copy link
Copy Markdown

@xodn348 xodn348 commented May 1, 2026

Summary

PeakSignalNoiseRatioWithBlockedEffect.__init__ contains a one-character logic error in its input guard: the condition uses and where or is required.

# Before (broken)
if not isinstance(block_size, int) and block_size < 1:

# After (correct)
if not isinstance(block_size, int) or block_size < 1:

With and, the first clause (not isinstance(block_size, int)) evaluates to False for every int, which short-circuits the expression and skips the < 1 check entirely. As a result:

  • block_size=0 and block_size=-5 are silently accepted instead of raising ValueError.
  • block_size=1.5 is silently accepted (float >= 1 makes the second clause False).
  • block_size="foo" raises TypeError from the comparison instead of the intended ValueError.

This PR applies the same or-based guard to the functional peak_signal_noise_ratio_with_blocked_effect, which had no block_size validation at all — an omission noted in the issue thread.

Issue

Fixes #3364

Local verification

# Linter
$ ruff check src/torchmetrics/image/psnrb.py \
             src/torchmetrics/functional/image/psnrb.py \
             tests/unittests/image/test_psnrb.py
All checks passed!

# Tests (9 new + 1 existing)
$ python -m pytest \
    tests/unittests/image/test_psnrb.py::test_error_on_invalid_block_size_class \
    tests/unittests/image/test_psnrb.py::test_error_on_invalid_block_size_functional \
    tests/unittests/image/test_psnrb.py::test_error_on_color_images \
    -v

tests/unittests/image/test_psnrb.py::test_error_on_invalid_block_size_class[0]    PASSED
tests/unittests/image/test_psnrb.py::test_error_on_invalid_block_size_class[-5]   PASSED
tests/unittests/image/test_psnrb.py::test_error_on_invalid_block_size_class[1.5]  PASSED
tests/unittests/image/test_psnrb.py::test_error_on_invalid_block_size_class[foo]  PASSED
tests/unittests/image/test_psnrb.py::test_error_on_invalid_block_size_functional[0]    PASSED
tests/unittests/image/test_psnrb.py::test_error_on_invalid_block_size_functional[-5]   PASSED
tests/unittests/image/test_psnrb.py::test_error_on_invalid_block_size_functional[1.5]  PASSED
tests/unittests/image/test_psnrb.py::test_error_on_invalid_block_size_functional[foo]  PASSED
tests/unittests/image/test_psnrb.py::test_error_on_color_images                        PASSED

9 passed in 2.99s
=== LOCAL_TEST_PASSED ===

Risk

The change only affects the error-path for invalid block_size arguments — no functional computation is touched. Users passing a valid positive integer (block_size >= 1) are unaffected. The only observable behavioral change is that previously-ignored bad inputs now raise ValueError as documented, which is the intended contract. No existing tests regress.


📚 Documentation preview 📚: https://torchmetrics--3367.org.readthedocs.build/en/3367/

…ioWithBlockedEffect

The `and` operator in the guard was logically incorrect: for any int input
the first clause (`not isinstance(block_size, int)`) is False, which short-
circuits the expression so the `< 1` range check is never reached.  Zero
and negative ints slip through silently, floats slip through when >= 1, and
non-numeric types raise TypeError instead of the intended ValueError.

Change `and` to `or` so that the check raises ValueError for every value
that is not a positive integer, and add the same guard to the functional
`peak_signal_noise_ratio_with_blocked_effect` which had no validation at all.

Also fix a pre-existing RUF043 lint warning in the same test file (missing
raw-string prefix on a regex match= pattern).

Fixes Lightning-AI#3364
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PSNRB: block_size input validation is broken

1 participant