Skip to content

fix(validation): correct argument validation in auroc, logauc, and pesq metrics - #3441

Open
AbdullahRasheed45 wants to merge 1 commit into
Lightning-AI:masterfrom
AbdullahRasheed45:fix/arg-validation-auroc-logauc-pesq
Open

fix(validation): correct argument validation in auroc, logauc, and pesq metrics#3441
AbdullahRasheed45 wants to merge 1 commit into
Lightning-AI:masterfrom
AbdullahRasheed45:fix/arg-validation-auroc-logauc-pesq

Conversation

@AbdullahRasheed45

Copy link
Copy Markdown

Summary

Four more instances of the same and vs or logical operator bug in argument validation, affecting the AUROC, LogAUC, and PESQ metrics. These are the remaining sites not covered by PR #3440.

The Bug

Each guard uses and where or is required:

# Wrong: only raises when BOTH conditions are simultaneously True
if <type-check-fails> and <value-check-fails>:
    raise ValueError(...)

This means a value with a correct type but invalid range silently bypasses the check.

Files Changed

File Parameter Example of bypassed invalid input
audio/pesq.py n_processes n_processes=0 (int, but ≤ 0)
functional/classification/auroc.py max_fpr max_fpr=2.0 (float, but outside (0, 1])
retrieval/auroc.py max_fpr same as above in class API
functional/classification/logauc.py fpr_range fpr_range=[0.001, 0.1] (list of len 2, not tuple)

Reproduction

from torchmetrics.functional.classification import binary_auroc

# Before fix: no error for out-of-range max_fpr (silently computes wrong AUROC)
binary_auroc(preds, target, max_fpr=2.0)

# After fix:
# ValueError: Arguments `max_fpr` should be a float in range (0, 1], but got: 2.0
from torchmetrics.audio import PerceptualEvaluationSpeechQuality

# Before fix: no error for n_processes=0
PerceptualEvaluationSpeechQuality(fs=16000, mode='wb', n_processes=0)

# After fix:
# ValueError: Expected argument `n_processes` to be an int larger than 0 but got 0

Related

…sq metrics

Four more instances of the same 'and' vs 'or' logical operator bug found in
the broader codebase, affecting different metric families.

- PerceptualEvaluationSpeechQuality (audio/pesq.py):
  n_processes=0 or a negative integer was accepted without raising an error.

- BinaryAUROC functional API (functional/classification/auroc.py):
  max_fpr as an out-of-range float (e.g. 2.0) was accepted without error
  because the condition was 'not isinstance(float) AND in range (0,1]'
  instead of 'not isinstance(float) OR not in range (0,1]'.

- RetrievalAUROC class API (retrieval/auroc.py):
  Same max_fpr bug as above in the class __init__.

- BinaryLogAUC / MulticlassLogAUC / MultilabelLogAUC
  (functional/classification/logauc.py):
  fpr_range validation used 'and' instead of 'or', so a list of length 2
  would bypass the tuple type check.

Fix: replace 'and' with 'or' (and restructure the max_fpr check using De
Morgan's law) so that an invalid argument raises ValueError when either the
type check or the value check fails.
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.

1 participant