diff --git a/CHANGELOG.md b/CHANGELOG.md index 756d3587..b9f2bee6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,25 @@ All notable changes to this project will be documented in this file. +## 1.5.1 + +### Added + +- Model evaluation comparison for a dataset version + ([#529](https://github.com/roboflow/roboflow-python/pull/529)): + - `Workspace.compare_model_evaluations(project, version, frontier_metric=None)` + — compare test-set accuracy and median latency, including Pareto frontier + membership and reasons models are excluded. + - `roboflow --workspace eval compare --project --version ` + — display the comparison as a table; use `--json` for the public API response. + Use `--frontier-metric` to choose the metric for frontier membership. + - Both surfaces read existing results without starting evaluations. + +### Changed + +- All `roboflow eval` commands now return exit code `2` for HTTP 401/403 + access errors (previously `1`). + ## 1.5.0 ### Added diff --git a/CLI-COMMANDS.md b/CLI-COMMANDS.md index 10ec349a..bd5c619e 100644 --- a/CLI-COMMANDS.md +++ b/CLI-COMMANDS.md @@ -125,6 +125,7 @@ roboflow --json train results my-project/3 | jq -r .modelGroup roboflow model list -p my-project --group rfdetrNasGroup-3 # Star a NAS-trained model (triggers TRT compile for its recommended hardware): +# Also starts model evaluation when the workspace has Model Evaluation access. # --json train results … gives you the modelId per row. roboflow model star roboflow model star --unstar @@ -385,11 +386,25 @@ roboflow eval recommendations --json ``` Exit codes are stable per error class so scripts and agents can react -without parsing message strings: `3` for `model_eval_not_found` (404), +without parsing message strings: `2` for authentication or access errors (401/403), +`3` for `model_eval_not_found` (404), `4` for `model_eval_not_done` (409 — eval still running), `5` for `invalid_split` / `invalid_confidence` (400). Requires the `model-eval:read` scope on the api key. +### Compare Models + +```bash +# Compare accuracy and latency for one version. +roboflow --workspace my-workspace eval compare --project my-project --version 3 + +# Choose the Pareto frontier metric; return all metrics and exclusion reasons. +roboflow eval compare --project my-project --version 3 --frontier-metric mAP5095 --json +``` + +Reads existing evaluations; does not start new ones. Requires `model-eval:read` +and workspace Model Evaluation access. + ### Workspace stats and billing ```bash diff --git a/roboflow/__init__.py b/roboflow/__init__.py index 70f793eb..6c556500 100644 --- a/roboflow/__init__.py +++ b/roboflow/__init__.py @@ -21,7 +21,7 @@ CLIPModel = None # type: ignore[assignment,misc] GazeModel = None # type: ignore[assignment,misc] -__version__ = "1.5.0" +__version__ = "1.5.1" def check_key(api_key, model, notebook, num_retries=0): diff --git a/roboflow/adapters/rfapi.py b/roboflow/adapters/rfapi.py index e29b2b2e..72f88701 100644 --- a/roboflow/adapters/rfapi.py +++ b/roboflow/adapters/rfapi.py @@ -2110,6 +2110,10 @@ class ModelEvalNotDoneError(RoboflowError): """Raised when reading panel data for an eval whose status is not ``done`` (HTTP 409).""" +class ModelEvalAccessError(RoboflowError): + """Raised when an evaluation read is not authorized (HTTP 401 or 403).""" + + class InvalidSplitError(RoboflowError): """Raised when ``split`` is not one of the accepted values (HTTP 400).""" @@ -2149,6 +2153,8 @@ def _model_eval_error_for(response): "invalid_confidence": InvalidConfidenceError, } cls = cls_by_code.get(code or "") + if response.status_code in (401, 403): + return ModelEvalAccessError(message) if cls is not None: return cls(message) if response.status_code == 404: @@ -2172,6 +2178,23 @@ def _eval_get(api_key, workspace_url, path, params=None): return response.json() +def compare_model_evals( + api_key: str, + workspace_url: str, + *, + project: str, + version: Union[str, int], + frontier_metric: Optional[str] = None, +) -> dict: + """GET /{workspace}/model-evals/compare — compare models on a dataset version.""" + return _eval_get( + api_key, + workspace_url, + "/compare", + params={"project": project, "version": version, "frontierMetric": frontier_metric}, + ) + + def list_model_evals( api_key: str, workspace_url: str, diff --git a/roboflow/cli/handlers/eval.py b/roboflow/cli/handlers/eval.py index 0e41cc90..e55d556d 100644 --- a/roboflow/cli/handlers/eval.py +++ b/roboflow/cli/handlers/eval.py @@ -24,6 +24,24 @@ # --------------------------------------------------------------------------- +@eval_app.command("compare") +def compare_evals_cmd( + ctx: typer.Context, + project: Annotated[str, typer.Option("-p", "--project", help="Project slug")], + version: Annotated[int, typer.Option("-v", "--version", min=1, help="Dataset version number")], + frontier_metric: Annotated[ + Optional[str], + typer.Option( + "--frontier-metric", + help="Frontier metric: mAP, mAP5095, mAP75, mIoU, precision, recall, f1 (default: project default)", + ), + ] = None, +) -> None: + """Compare test-set accuracy and median latency without starting evaluations.""" + args = ctx_to_args(ctx, project=project, version=version, frontier_metric=frontier_metric) + _compare_evals(args) + + @eval_app.command("list") def list_evals_cmd( ctx: typer.Context, @@ -171,6 +189,8 @@ def _eval_error_exit_code(exc: Exception) -> int: """ from roboflow.adapters import rfapi + if isinstance(exc, rfapi.ModelEvalAccessError): + return 2 if isinstance(exc, rfapi.ModelEvalNotFoundError): return 3 if isinstance(exc, rfapi.ModelEvalNotDoneError): @@ -184,6 +204,8 @@ def _hint_for(exc: Exception) -> Optional[str]: """Per-error actionable hint shown alongside the message in non-JSON mode.""" from roboflow.adapters import rfapi + if isinstance(exc, rfapi.ModelEvalAccessError): + return "Check your API key, its model-eval:read scope, and workspace Model Evaluation access." if isinstance(exc, rfapi.ModelEvalNotFoundError): return "Run 'roboflow eval list' to see eval ids in this workspace." if isinstance(exc, rfapi.ModelEvalNotDoneError): @@ -195,6 +217,60 @@ def _hint_for(exc: Exception) -> Optional[str]: return None +def _compare_evals(args): # noqa: ANN001 + from roboflow.adapters import rfapi + from roboflow.cli._output import output, output_error + from roboflow.cli._table import format_table + + resolved = _resolve(args) + if not resolved: + return + workspace_url, api_key = resolved + try: + comparison = rfapi.compare_model_evals( + api_key, + workspace_url, + project=args.project, + version=args.version, + frontier_metric=args.frontier_metric, + ) + except Exception as exc: + output_error( + args, + str(exc), + hint=( + _hint_for(exc) + if isinstance(exc, rfapi.ModelEvalAccessError) + else "Check the project, version, frontier metric, and workspace access." + ), + exit_code=_eval_error_exit_code(exc), + ) + return + if args.json: + output(args, comparison) + return + frontier_metric = comparison.get("frontierMetric") + rows = [] + for model in comparison.get("models", []): + accuracy = model.get("metrics", {}).get(frontier_metric) if frontier_metric else None + latency = model.get("medianLatencyMs") + rows.append( + { + "model": model["modelId"], + "accuracy": f"{accuracy * 100:.1f}%" if accuracy is not None else "", + "latency": f"{latency:.2f}" if latency is not None else "", + "frontier": "Yes" if model.get("onFrontier") else "", + "exclusion": model.get("exclusionReason") or "", + } + ) + table = format_table( + rows, + columns=["model", "accuracy", "latency", "frontier", "exclusion"], + headers=["MODEL", frontier_metric or "ACCURACY", "MEDIAN LATENCY (ms)", "FRONTIER", "EXCLUSION"], + ) + output(args, comparison, text=table) + + def _list_evals(args): # noqa: ANN001 from roboflow.adapters import rfapi from roboflow.cli._output import output, output_error diff --git a/roboflow/core/workspace.py b/roboflow/core/workspace.py index 69fd055f..d4e3a634 100644 --- a/roboflow/core/workspace.py +++ b/roboflow/core/workspace.py @@ -8,7 +8,7 @@ import tempfile import time import zipfile -from typing import TYPE_CHECKING, Any, Dict, Generator, List, Optional +from typing import TYPE_CHECKING, Any, Dict, Generator, List, Optional, Union import requests from requests.exceptions import HTTPError @@ -1572,6 +1572,32 @@ def upload_vision_event_image( # Model evaluations # ----------------------------------------------------------------- + def compare_model_evaluations( + self, + project: str, + version: Union[str, int], + *, + frontier_metric: Optional[str] = None, + ) -> dict: + """Compare model accuracy and median latency for a dataset version. + + Args: + project: Project URL slug. + version: Dataset version number. + frontier_metric: Metric for frontier membership. The server uses + the project default when this value is not specified. + + Returns: + The public model comparison response. + """ + return rfapi.compare_model_evals( + self.__api_key, + self.url, + project=project, + version=version, + frontier_metric=frontier_metric, + ) + def evals( self, *, diff --git a/tests/adapters/test_rfapi_model_evals.py b/tests/adapters/test_rfapi_model_evals.py index 41cc6942..b758d906 100644 --- a/tests/adapters/test_rfapi_model_evals.py +++ b/tests/adapters/test_rfapi_model_evals.py @@ -220,5 +220,31 @@ def test_non_json_body_falls_back_to_text(self, mock_get): self.assertIn("Bad Gateway", str(ctx.exception)) +class TestCompareModelEvals(unittest.TestCase): + @patch("roboflow.adapters.rfapi.requests.get") + def test_compare_returns_public_result_and_sends_frontier_metric(self, mock_get): + comparison = { + "project": "chess", + "version": "131", + "frontierMetric": "mAP5095", + "availableMetrics": ["mAP", "mAP5095"], + "models": [], + } + mock_get.return_value = _resp(200, comparison) + + result = rfapi.compare_model_evals("k", "ws", project="chess", version="131", frontier_metric="mAP5095") + + self.assertEqual(result, comparison) + mock_get.assert_called_once_with( + f"{API_URL}/ws/model-evals/compare", + params={ + "api_key": "k", + "project": "chess", + "version": "131", + "frontierMetric": "mAP5095", + }, + ) + + if __name__ == "__main__": unittest.main() diff --git a/tests/cli/test_eval_handler.py b/tests/cli/test_eval_handler.py index 0d4a4a30..4368d2ef 100644 --- a/tests/cli/test_eval_handler.py +++ b/tests/cli/test_eval_handler.py @@ -5,7 +5,8 @@ import json import unittest from argparse import Namespace -from unittest.mock import patch +from pathlib import Path +from unittest.mock import MagicMock, patch from typer.testing import CliRunner @@ -397,6 +398,7 @@ def test_exit_codes(self) -> None: from roboflow.cli.handlers.eval import _eval_error_exit_code cases = { + rfapi.ModelEvalAccessError("x"): 2, rfapi.ModelEvalNotFoundError("x"): 3, rfapi.ModelEvalNotDoneError("x"): 4, rfapi.InvalidSplitError("x"): 5, @@ -409,5 +411,165 @@ def test_exit_codes(self) -> None: self.assertEqual(_eval_error_exit_code(exc), expected) +class TestEvalCompareCommand(unittest.TestCase): + @patch("roboflow.adapters.rfapi.requests.get") + def test_not_found_is_a_structured_error(self, mock_get): + mock_get.return_value = MagicMock(status_code=404, text="Project not found") + mock_get.return_value.json.return_value = {"error": "Project not found"} + result = runner.invoke( + app, + ["--api-key", "k", "--workspace", "ws", "--json", "eval", "compare", "-p", "chess", "-v", "131"], + ) + + self.assertEqual(result.exit_code, 3) + self.assertEqual(json.loads(result.stderr)["error"]["message"], "Project not found") + self.assertEqual( + json.loads(result.stderr)["error"]["hint"], + "Check the project, version, frontier metric, and workspace access.", + ) + self.assertEqual(result.stdout, "") + + @patch("roboflow.adapters.rfapi.requests.get") + def test_null_frontier_metric_shows_exclusion_without_accuracy(self, mock_get): + mock_get.return_value = MagicMock(status_code=200) + mock_get.return_value.json.return_value = { + "frontierMetric": None, + "models": [ + { + "modelId": "ws/chess-model", + "metrics": {"mAP": 0.9}, + "medianLatencyMs": 10, + "onFrontier": False, + "exclusionReason": "unsupported_project_type", + } + ], + } + result = runner.invoke( + app, + ["--api-key", "k", "--workspace", "ws", "eval", "compare", "-p", "chess", "-v", "131"], + ) + + self.assertEqual(result.exit_code, 0, result.output) + for text in ["ACCURACY", "ws/chess-model", "10.00", "unsupported_project_type"]: + self.assertIn(text, result.stdout) + self.assertNotIn("90.0%", result.stdout) + + @patch("roboflow.adapters.rfapi.requests.get") + def test_list_access_error_includes_auth_and_entitlement_hint(self, mock_get): + mock_get.return_value = MagicMock(status_code=403, text="Access denied") + mock_get.return_value.json.return_value = {"error": "Access denied"} + result = runner.invoke(app, ["--api-key", "k", "--workspace", "ws", "--json", "eval", "list"]) + + self.assertEqual(result.exit_code, 2) + hint = json.loads(result.stderr)["error"]["hint"] + for text in ["API key", "model-eval:read", "Model Evaluation access"]: + self.assertIn(text, hint) + + @patch("roboflow.adapters.rfapi.requests.get") + def test_json_preserves_the_full_comparison(self, mock_get): + comparison = json.loads((Path(__file__).parents[1] / "fixtures/model_eval_comparison.json").read_text()) + mock_get.return_value = MagicMock(status_code=200) + mock_get.return_value.json.return_value = comparison + + result = runner.invoke( + app, + [ + "--api-key", + "k", + "--workspace", + "ws", + "--json", + "eval", + "compare", + "--project", + "chess", + "--version", + "131", + "--frontier-metric", + "mAP5095", + ], + ) + + self.assertEqual(result.exit_code, 0, result.output) + self.assertEqual(json.loads(result.stdout), comparison) + self.assertEqual(mock_get.call_count, 1) + self.assertEqual(mock_get.call_args.kwargs["params"]["frontierMetric"], "mAP5095") + + @patch("roboflow.adapters.rfapi.requests.get") + def test_text_shows_server_frontier_and_exclusion_with_zero_values(self, mock_get): + mock_get.return_value = MagicMock(status_code=200) + mock_get.return_value.json.return_value = { + "project": "chess", + "version": "131", + "frontierMetric": "mAP", + "availableMetrics": ["mAP"], + "models": [ + { + "modelId": "ws/chess-fast", + "evaluationId": "eval-fast", + "metrics": {"mAP": 0}, + "medianLatencyMs": 0, + "onFrontier": True, + "exclusionReason": None, + }, + { + "modelId": "ws/chess-old", + "evaluationId": "eval-old", + "metrics": {"mAP": 0.9}, + "medianLatencyMs": None, + "onFrontier": False, + "exclusionReason": "latency_unavailable", + }, + ], + } + result = runner.invoke( + app, + ["--api-key", "k", "--workspace", "ws", "eval", "compare", "--project", "chess", "--version", "131"], + ) + + self.assertEqual(result.exit_code, 0, result.output) + for text in [ + "MODEL", + "mAP", + "MEDIAN LATENCY (ms)", + "FRONTIER", + "EXCLUSION", + "ws/chess-fast", + "0.0%", + "0.00", + "Yes", + "latency_unavailable", + ]: + self.assertIn(text, result.stdout) + + @patch("roboflow.adapters.rfapi.requests.get") + def test_permission_failure_is_a_structured_auth_error(self, mock_get): + mock_get.return_value = MagicMock(status_code=403, text="Comparison access denied") + mock_get.return_value.json.return_value = {"error": "forbidden", "message": "Comparison access denied"} + result = runner.invoke( + app, + [ + "--api-key", + "k", + "--workspace", + "ws", + "--json", + "eval", + "compare", + "--project", + "chess", + "--version", + "131", + ], + ) + + self.assertEqual(result.exit_code, 2) + self.assertEqual(json.loads(result.stderr)["error"]["message"], "Comparison access denied") + hint = json.loads(result.stderr)["error"]["hint"] + for text in ["API key", "model-eval:read", "Model Evaluation access"]: + self.assertIn(text, hint) + self.assertEqual(result.stdout, "") + + if __name__ == "__main__": unittest.main() diff --git a/tests/fixtures/model_eval_comparison.json b/tests/fixtures/model_eval_comparison.json new file mode 100644 index 00000000..909ea58e --- /dev/null +++ b/tests/fixtures/model_eval_comparison.json @@ -0,0 +1,63 @@ +{ + "project": "chess", + "version": "131", + "frontierMetric": "mAP", + "availableMetrics": [ + "mAP", + "mAP5095", + "mAP75", + "precision", + "recall", + "f1" + ], + "models": [ + { + "modelId": "acme/chess-fast", + "evaluationId": "eval-fast", + "metrics": { + "mAP": 0.8, + "mIoU": null, + "f1": 0.8, + "precision": 0.8, + "recall": 0.8, + "mAP5095": 0.8, + "mAP75": 0.8 + }, + "medianLatencyMs": 3, + "onFrontier": true, + "exclusionReason": null + }, + { + "modelId": "acme/chess-dominated", + "evaluationId": "eval-dominated", + "metrics": { + "mAP": 0.7, + "mIoU": null, + "f1": 0.7, + "precision": 0.7, + "recall": 0.7, + "mAP5095": 0.7, + "mAP75": 0.7 + }, + "medianLatencyMs": 5, + "onFrontier": false, + "exclusionReason": null + }, + { + "modelId": "acme/chess-missing-latency", + "evaluationId": "eval-missing-latency", + "metrics": { + "mAP": 0.9, + "mIoU": null, + "f1": 0.9, + "precision": 0.9, + "recall": 0.9, + "mAP5095": 0.9, + "mAP75": 0.9 + }, + "medianLatencyMs": null, + "onFrontier": false, + "exclusionReason": "latency_unavailable" + } + ] +} diff --git a/tests/test_model_eval.py b/tests/test_model_eval.py index 40072bab..31c5fec6 100644 --- a/tests/test_model_eval.py +++ b/tests/test_model_eval.py @@ -246,6 +246,28 @@ def test_refresh_404_propagates(self, mock_fn): class TestWorkspaceEvalAccessors(unittest.TestCase): + @patch("roboflow.adapters.rfapi.compare_model_evals") + def test_compare_model_evaluations_returns_public_comparison(self, mock_compare): + comparison = { + "project": "chess", + "version": "131", + "frontierMetric": "mAP5095", + "availableMetrics": ["mAP", "mAP5095"], + "models": [], + } + mock_compare.return_value = comparison + + result = _make_workspace().compare_model_evaluations("chess", 131, frontier_metric="mAP5095") + + self.assertEqual(result, comparison) + mock_compare.assert_called_once_with( + "k", + "lee-sandbox", + project="chess", + version=131, + frontier_metric="mAP5095", + ) + @patch("roboflow.adapters.rfapi.list_model_evals") def test_evals_returns_modeleval_instances(self, mock_list): from roboflow.core.model_eval import ModelEval