From 66d4ddce1c1386cf782c53949852ea4b6f622d24 Mon Sep 17 00:00:00 2001 From: AbdullahRasheed45 Date: Fri, 31 Jul 2026 03:21:14 +0100 Subject: [PATCH] fix(validation): correct argument validation in auroc, logauc, and pesq 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. --- src/torchmetrics/audio/pesq.py | 2 +- src/torchmetrics/functional/classification/auroc.py | 2 +- src/torchmetrics/functional/classification/logauc.py | 2 +- src/torchmetrics/retrieval/auroc.py | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/torchmetrics/audio/pesq.py b/src/torchmetrics/audio/pesq.py index 38146f6c943..deef13398c4 100644 --- a/src/torchmetrics/audio/pesq.py +++ b/src/torchmetrics/audio/pesq.py @@ -114,7 +114,7 @@ def __init__( if mode not in ("wb", "nb"): raise ValueError(f"Expected argument `mode` to either be 'wb' or 'nb' but got {mode}") self.mode = mode - if not isinstance(n_processes, int) and n_processes <= 0: + if not isinstance(n_processes, int) or n_processes <= 0: raise ValueError(f"Expected argument `n_processes` to be an int larger than 0 but got {n_processes}") self.n_processes = n_processes diff --git a/src/torchmetrics/functional/classification/auroc.py b/src/torchmetrics/functional/classification/auroc.py index 1dfd752ee22..3be1c294824 100644 --- a/src/torchmetrics/functional/classification/auroc.py +++ b/src/torchmetrics/functional/classification/auroc.py @@ -76,7 +76,7 @@ def _binary_auroc_arg_validation( ignore_index: Optional[int] = None, ) -> None: _binary_precision_recall_curve_arg_validation(thresholds, ignore_index) - if max_fpr is not None and not isinstance(max_fpr, float) and 0 < max_fpr <= 1: + if max_fpr is not None and (not isinstance(max_fpr, float) or not (0 < max_fpr <= 1)): raise ValueError(f"Arguments `max_fpr` should be a float in range (0, 1], but got: {max_fpr}") diff --git a/src/torchmetrics/functional/classification/logauc.py b/src/torchmetrics/functional/classification/logauc.py index 17ff4bbbceb..2033f12c5c1 100644 --- a/src/torchmetrics/functional/classification/logauc.py +++ b/src/torchmetrics/functional/classification/logauc.py @@ -26,7 +26,7 @@ def _validate_fpr_range(fpr_range: Tuple[float, float]) -> None: """Validate the `fpr_range` argument for the logauc metric.""" - if not isinstance(fpr_range, tuple) and not len(fpr_range) == 2: + if not isinstance(fpr_range, tuple) or len(fpr_range) != 2: raise ValueError(f"The `fpr_range` should be a tuple of two floats, but got {type(fpr_range)}.") if not (0 <= fpr_range[0] < fpr_range[1] <= 1): raise ValueError(f"The `fpr_range` should be a tuple of two floats in the range [0, 1], but got {fpr_range}.") diff --git a/src/torchmetrics/retrieval/auroc.py b/src/torchmetrics/retrieval/auroc.py index 6d4382894a4..52e2651c343 100644 --- a/src/torchmetrics/retrieval/auroc.py +++ b/src/torchmetrics/retrieval/auroc.py @@ -114,7 +114,7 @@ def __init__( if top_k is not None and not (isinstance(top_k, int) and top_k > 0): raise ValueError("`top_k` has to be a positive integer or None") self.top_k = top_k - if max_fpr is not None and not isinstance(max_fpr, float) and 0 < max_fpr <= 1: + if max_fpr is not None and (not isinstance(max_fpr, float) or not (0 < max_fpr <= 1)): raise ValueError(f"Arguments `max_fpr` should be a float in range (0, 1], but got: {max_fpr}") self.max_fpr = max_fpr