From 74db9e29ffc059b61573d617141bea764050ff11 Mon Sep 17 00:00:00 2001 From: Jirka Borovec <6035284+Borda@users.noreply.github.com> Date: Tue, 7 Jul 2026 21:59:23 +0200 Subject: [PATCH] fix(utils): normalize timm confidences and verify assets (#2414) - Convert timm classification logits with softmax so confidence values match the normalized scale used by other classification adapters. - Verify asset MD5 hashes after fresh downloads and retry once when a payload is corrupted. - Add focused regressions for timm confidence scaling and asset download integrity paths. - Convert from_timm outputs to probabilities before applying thresholds and document that existing thresholds may need retuning. - Add downloader regression coverage for repeated MD5 mismatches so exhausted retries now raise ValueError. --------- Co-authored-by: Codex --- docs/changelog.md | 2 + src/supervision/assets/downloader.py | 65 +++++++++--- src/supervision/classification/core.py | 8 +- tests/assets/test_downloader.py | 138 ++++++++++++++++++++++++- tests/classification/test_core.py | 28 ++++- 5 files changed, 220 insertions(+), 21 deletions(-) diff --git a/docs/changelog.md b/docs/changelog.md index 4532ea17..83130d28 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -18,6 +18,8 @@ date_modified: 2026-07-06 - `sv.mask_non_max_merge` now computes exact mask overlap at the original mask resolution and ignores the deprecated `mask_dimension` parameter. Code that relied on downscaled mask overlap should recalibrate thresholds; passing `mask_dimension` positionally now emits a deprecation warning, and the parameter is scheduled for removal in `0.33.0` ([#2400](https://github.com/roboflow/supervision/pull/2400)). ### Fixed +- `sv.Classifications.from_timm` now softmaxes model logits before exposing confidence scores, matching `sv.Classifications.from_clip` and keeping timm confidences on a normalized probability scale. Thresholds calibrated against raw logits may need retuning. +- `sv.download_assets` now verifies MD5 hashes after fresh downloads and retries once when the downloaded payload is corrupted instead of accepting a bad file. - Fixed metrics scoring edge cases: legacy `sv.MeanAveragePrecision` now uses COCO 101-point AP averaging, `sv.ConfusionMatrix` rejects invalid class ids instead of wrapping them through `int16`/negative indexing, `sv.MeanAveragePrecision` preserves user-provided target `ignore` flags, and `sv.MeanAverageRecallResult.recall_per_class` now exposes per-class recall for each max-detection cutoff. - `sv.ByteTrack` no longer mutates input `Detections` while assigning tracker IDs. It now keeps detections at the activation-threshold boundary eligible for matching, avoids impossible new-track thresholds above score `1.0`, ignores invalid zero-area/non-finite tensor boxes before Kalman updates, and does not emit unconfirmed `-1` IDs from first-frame tensor updates. - Fixed [#2402](https://github.com/roboflow/supervision/pull/2402): `sv.KeyPoints.as_detections` now accepts NumPy arrays, tuples, and generators in `selected_keypoint_indices` without ambiguous truth-value errors; empty index iterables select all keypoints. Valid zero-area skeletons are preserved, while all-zero and non-finite-only skeletons are filtered out. diff --git a/src/supervision/assets/downloader.py b/src/supervision/assets/downloader.py index 19cd7442..1cad7646 100644 --- a/src/supervision/assets/downloader.py +++ b/src/supervision/assets/downloader.py @@ -36,6 +36,51 @@ def is_md5_hash_matching(filename: str, original_md5_hash: str) -> bool: return computed_md5_hash.hexdigest() == original_md5_hash +def _download_asset(filename: str) -> None: + """ + Download asset bytes to the target filename. + """ + response = get( + MEDIA_ASSETS[filename][0], stream=True, allow_redirects=True, timeout=30 + ) + response.raise_for_status() + + file_size = int(response.headers.get("Content-Length", 0)) + folder_path = Path(filename).expanduser().resolve() + folder_path.parent.mkdir(parents=True, exist_ok=True) + + with tqdm.wrapattr( + response.raw, "read", total=file_size, desc="", colour="#a351fb" + ) as raw_resp: + with folder_path.open("wb") as file: + copyfileobj(raw_resp, file) + + +def _download_verified_asset( + filename: str, original_md5_hash: str, retry_on_mismatch: bool = True +) -> None: + """ + Download an asset and reject payloads whose MD5 does not match the catalog. + """ + _download_asset(filename) + + if is_md5_hash_matching(filename, original_md5_hash): + return + + logger.warning("File corrupted. Re-downloading...") + os.remove(filename) + + if retry_on_mismatch: + _download_verified_asset( + filename=filename, + original_md5_hash=original_md5_hash, + retry_on_mismatch=False, + ) + return + + raise ValueError(f"Downloaded asset {filename!r} failed MD5 verification.") + + def download_assets(asset_name: Assets | str) -> str: """ Download a specified asset if it doesn't already exist or is corrupted. @@ -61,27 +106,15 @@ def download_assets(asset_name: Assets | str) -> str: filename = asset_name.filename if isinstance(asset_name, Assets) else asset_name if filename in MEDIA_ASSETS: + original_md5_hash = MEDIA_ASSETS[filename][1] if not Path(filename).exists(): logger.info("Downloading %s assets", filename) - response = get( - MEDIA_ASSETS[filename][0], stream=True, allow_redirects=True, timeout=30 - ) - response.raise_for_status() - - file_size = int(response.headers.get("Content-Length", 0)) - folder_path = Path(filename).expanduser().resolve() - folder_path.parent.mkdir(parents=True, exist_ok=True) - - with tqdm.wrapattr( - response.raw, "read", total=file_size, desc="", colour="#a351fb" - ) as raw_resp: - with folder_path.open("wb") as file: - copyfileobj(raw_resp, file) + _download_verified_asset(filename, original_md5_hash) else: - if not is_md5_hash_matching(filename, MEDIA_ASSETS[filename][1]): + if not is_md5_hash_matching(filename, original_md5_hash): logger.warning("File corrupted. Re-downloading...") os.remove(filename) - return download_assets(filename) + _download_verified_asset(filename, original_md5_hash) logger.info("%s asset download complete.", filename) else: diff --git a/src/supervision/classification/core.py b/src/supervision/classification/core.py index e34c55c8..00789274 100644 --- a/src/supervision/classification/core.py +++ b/src/supervision/classification/core.py @@ -7,7 +7,7 @@ import numpy as np import numpy.typing as npt if TYPE_CHECKING: - import torch + import torch # type: ignore[import-not-found, unused-ignore] def _validate_class_ids(class_id: Any, n: int) -> None: @@ -123,6 +123,10 @@ class Classifications: Creates a Classifications instance from a [timm](https://huggingface.co/docs/hub/timm) inference result. + Note: + Returned confidences are softmax-normalized probabilities, so + thresholds calibrated against raw logits may need recalibration. + Args: timm_results: The inference result from timm model. @@ -152,7 +156,7 @@ class Classifications: classifications = sv.Classifications.from_timm(output) ``` """ - confidence = timm_results.cpu().detach().numpy()[0] + confidence = timm_results.softmax(dim=-1).cpu().detach().numpy()[0] if len(confidence) == 0: return cls( diff --git a/tests/assets/test_downloader.py b/tests/assets/test_downloader.py index ca0c677d..c0c5aa0d 100644 --- a/tests/assets/test_downloader.py +++ b/tests/assets/test_downloader.py @@ -52,18 +52,48 @@ class TestDownloadAssets: "supervision.assets.downloader.is_md5_hash_matching", side_effect=[False, True], ) + @patch("pathlib.Path.open", new_callable=mock_open) + @patch("pathlib.Path.mkdir") @patch("pathlib.Path.exists", return_value=True) + @patch("supervision.assets.downloader.copyfileobj") + @patch("supervision.assets.downloader.tqdm") + @patch("supervision.assets.downloader.get") def test_already_exists_but_corrupted( - self, mock_exists, mock_md5, mock_remove, mock_logger + self, + mock_get, + mock_tqdm, + mock_copyfileobj, + mock_exists, + mock_mkdir, + mock_open_file, + mock_md5, + mock_remove, + mock_logger, ) -> None: """Test download_assets when file exists but is corrupted (re-downloads).""" filename = "vehicles.mp4" + + mock_response = MagicMock() + mock_response.headers = {"Content-Length": "100"} + mock_response.raw = MagicMock() + mock_response.raise_for_status = MagicMock() + mock_get.return_value = mock_response + + mock_tqdm.wrapattr.return_value.__enter__ = MagicMock( + return_value=mock_response.raw + ) + mock_tqdm.wrapattr.return_value.__exit__ = MagicMock() + result = download_assets(filename) + assert result == filename + mock_get.assert_called_once() + mock_copyfileobj.assert_called_once() mock_logger.warning.assert_called_once_with("File corrupted. Re-downloading...") mock_remove.assert_called_once_with(filename) @patch("supervision.assets.downloader.logger") + @patch("supervision.assets.downloader.is_md5_hash_matching", return_value=True) @patch("pathlib.Path.open", new_callable=mock_open) @patch("pathlib.Path.mkdir") @patch("pathlib.Path.exists", return_value=False) @@ -78,9 +108,10 @@ class TestDownloadAssets: mock_exists, mock_mkdir, mock_open_file, + mock_md5, mock_logger, ) -> None: - """Test download_assets downloading a new file.""" + """Test download_assets verifies a freshly downloaded file.""" filename = "vehicles.mp4" mock_response = MagicMock() @@ -100,6 +131,99 @@ class TestDownloadAssets: mock_get.assert_called_once() mock_response.raise_for_status.assert_called_once_with() mock_copyfileobj.assert_called_once() + mock_md5.assert_called_once_with(filename, "8155ff4e4de08cfa25f39de96483f918") + + @patch("supervision.assets.downloader.logger") + @patch("os.remove") + @patch( + "supervision.assets.downloader.is_md5_hash_matching", + side_effect=[False, True], + ) + @patch("pathlib.Path.open", new_callable=mock_open) + @patch("pathlib.Path.mkdir") + @patch("pathlib.Path.exists", return_value=False) + @patch("supervision.assets.downloader.copyfileobj") + @patch("supervision.assets.downloader.tqdm") + @patch("supervision.assets.downloader.get") + def test_download_new_file_retries_corrupted_payload( + self, + mock_get, + mock_tqdm, + mock_copyfileobj, + mock_exists, + mock_mkdir, + mock_open_file, + mock_md5, + mock_remove, + mock_logger, + ) -> None: + """Test download_assets retries once when a fresh payload fails MD5.""" + filename = "vehicles.mp4" + + mock_response = MagicMock() + mock_response.headers = {"Content-Length": "100"} + mock_response.raw = MagicMock() + mock_response.raise_for_status = MagicMock() + mock_get.return_value = mock_response + + mock_tqdm.wrapattr.return_value.__enter__ = MagicMock( + return_value=mock_response.raw + ) + mock_tqdm.wrapattr.return_value.__exit__ = MagicMock() + + result = download_assets(filename) + + assert result == filename + assert mock_get.call_count == 2 + assert mock_copyfileobj.call_count == 2 + mock_remove.assert_called_once_with(filename) + mock_logger.warning.assert_called_once_with("File corrupted. Re-downloading...") + + @patch("supervision.assets.downloader.logger") + @patch("os.remove") + @patch( + "supervision.assets.downloader.is_md5_hash_matching", + side_effect=[False, False], + ) + @patch("pathlib.Path.open", new_callable=mock_open) + @patch("pathlib.Path.mkdir") + @patch("pathlib.Path.exists", return_value=False) + @patch("supervision.assets.downloader.copyfileobj") + @patch("supervision.assets.downloader.tqdm") + @patch("supervision.assets.downloader.get") + def test_download_new_file_raises_after_second_md5_mismatch( + self, + mock_get, + mock_tqdm, + mock_copyfileobj, + mock_exists, + mock_mkdir, + mock_open_file, + mock_md5, + mock_remove, + mock_logger, + ) -> None: + """Test download_assets fails after the verified retry is also corrupted.""" + filename = "vehicles.mp4" + + mock_response = MagicMock() + mock_response.headers = {"Content-Length": "100"} + mock_response.raw = MagicMock() + mock_response.raise_for_status = MagicMock() + mock_get.return_value = mock_response + + mock_tqdm.wrapattr.return_value.__enter__ = MagicMock( + return_value=mock_response.raw + ) + mock_tqdm.wrapattr.return_value.__exit__ = MagicMock() + + with pytest.raises(ValueError, match="failed MD5 verification"): + download_assets(filename) + + assert mock_get.call_count == 2 + assert mock_copyfileobj.call_count == 2 + assert mock_remove.call_count == 2 + assert mock_logger.warning.call_count == 2 @patch("pathlib.Path.exists", return_value=False) def test_invalid_asset(self, mock_exists) -> None: @@ -124,6 +248,7 @@ class TestDownloadAssets: assert "vehicles.mp4" in str(exc_info.value) @patch("supervision.assets.downloader.logger") + @patch("supervision.assets.downloader.is_md5_hash_matching", return_value=True) @patch("pathlib.Path.open", new_callable=mock_open) @patch("pathlib.Path.mkdir") @patch("supervision.assets.downloader.copyfileobj") @@ -138,6 +263,7 @@ class TestDownloadAssets: mock_copyfileobj, mock_mkdir, mock_open_file, + mock_md5, mock_logger, ) -> None: """Test download_assets with VideoAssets enum.""" @@ -155,8 +281,12 @@ class TestDownloadAssets: result = download_assets(asset) assert result == asset.filename mock_logger.info.assert_called_with("Downloading %s assets", asset.filename) + mock_md5.assert_called_once_with( + asset.filename, "8155ff4e4de08cfa25f39de96483f918" + ) @patch("supervision.assets.downloader.logger") + @patch("supervision.assets.downloader.is_md5_hash_matching", return_value=True) @patch("pathlib.Path.open", new_callable=mock_open) @patch("pathlib.Path.mkdir") @patch("supervision.assets.downloader.copyfileobj") @@ -171,6 +301,7 @@ class TestDownloadAssets: mock_copyfileobj, mock_mkdir, mock_open_file, + mock_md5, mock_logger, ) -> None: """Test download_assets with ImageAssets enum.""" @@ -188,3 +319,6 @@ class TestDownloadAssets: result = download_assets(asset) assert result == asset.filename mock_logger.info.assert_called_with("Downloading %s assets", asset.filename) + mock_md5.assert_called_once_with( + asset.filename, "0f5a4b98abf3e3973faf9e9260a7d876" + ) diff --git a/tests/classification/test_core.py b/tests/classification/test_core.py index 039b27c0..4fd2518d 100644 --- a/tests/classification/test_core.py +++ b/tests/classification/test_core.py @@ -9,19 +9,29 @@ from supervision.classification.core import Classifications class _MockTensor: + """Minimal tensor double that supports the tensor chain used by adapters.""" + def __init__(self, value: np.ndarray) -> None: self.value = value def softmax(self, dim: int) -> _MockTensor: - return self + """Return a tensor double with softmax applied along the requested axis.""" + if self.value.shape[dim] == 0: + return _MockTensor(self.value) + + exp = np.exp(self.value - np.max(self.value, axis=dim, keepdims=True)) + return _MockTensor(exp / np.sum(exp, axis=dim, keepdims=True)) def cpu(self) -> _MockTensor: + """Return the tensor double for chained CPU conversion calls.""" return self def detach(self) -> _MockTensor: + """Return the tensor double for chained graph-detach calls.""" return self def numpy(self) -> np.ndarray: + """Return the wrapped NumPy array.""" return self.value @@ -72,6 +82,7 @@ def test_top_k( expected_result: tuple[np.ndarray, np.ndarray] | None, exception: Exception, ) -> None: + """Retrieves requested top-k values or raises for malformed confidence input.""" with exception: result = Classifications( class_id=np.array(class_id), confidence=np.array(confidence) @@ -82,6 +93,7 @@ def test_top_k( def test_from_clip_empty_output_dtypes() -> None: + """Empty CLIP logits produce typed empty classification arrays.""" result = Classifications.from_clip(_MockTensor(np.empty((1, 0), dtype=np.float32))) assert result.class_id.dtype == np.int_ @@ -90,8 +102,22 @@ def test_from_clip_empty_output_dtypes() -> None: def test_from_timm_empty_output_dtypes() -> None: + """Empty timm logits produce typed empty classification arrays.""" result = Classifications.from_timm(_MockTensor(np.empty((1, 0), dtype=np.float32))) assert result.class_id.dtype == np.int_ assert result.confidence is not None assert result.confidence.dtype == np.float32 + + +def test_from_timm_softmaxes_logits() -> None: + """Timm logits are converted to normalized confidence scores.""" + logits = np.array([[0.0, 1.0, 2.0]], dtype=np.float32) + + result = Classifications.from_timm(_MockTensor(logits)) + + assert result.confidence is not None + assert np.allclose( + result.confidence, _MockTensor(logits).softmax(dim=-1).numpy()[0] + ) + assert np.isclose(np.sum(result.confidence), 1.0)