diff --git a/src/inference_endpoint/commands/benchmark/execute.py b/src/inference_endpoint/commands/benchmark/execute.py index a6ad406a5..c1894d2b2 100644 --- a/src/inference_endpoint/commands/benchmark/execute.py +++ b/src/inference_endpoint/commands/benchmark/execute.py @@ -1128,11 +1128,20 @@ def _summarize_and_log_metrics( logger.info(f"Completed in {perf_elapsed:.1f}s") if ctx.accuracy_only: - acc_total = sum( - ds.dataset.num_samples() * ds.num_repeats - for ds in ctx.eval_configs - if ds.dataset_type == DatasetType.ACCURACY - ) + # Count what was actually evaluated, not what was loaded. An external + # scorer (SKIP_ENDPOINT_PHASE) evaluates its own clamped subset -- + # SWE-bench Verified loads all 500 rows but scores `num_instances` of + # them -- and reporting the loaded size instead contradicts the + # per-dataset `unit=` line in the same summary. + acc_total = 0 + for ec in ctx.eval_configs: + if ec.dataset_type != DatasetType.ACCURACY: + continue + external = effective_external_sample_count(ec) + if external is not None: + acc_total += external + else: + acc_total += ec.dataset.num_samples() * ec.num_repeats logger.info(f"Accuracy-only: {acc_total} samples evaluated") else: logger.info( @@ -1208,6 +1217,53 @@ def finalize_benchmark(ctx: BenchmarkContext, bench: BenchmarkResult) -> None: # after the report artifacts so a write failure here can't discard them. write_accuracy_results(ctx.report_dir, accuracy_scores) + # Every artifact is on disk; only now may the run be declared a failure. + _require_accuracy_numbers(ctx, accuracy_scores) + + +def _require_accuracy_numbers( + ctx: BenchmarkContext, accuracy_scores: list[dict[str, Any]] +) -> None: + """Fail a run that was asked for accuracy and produced no number. + + A scorer can complete every unit of work, exit cleanly, and still hand back + ``score=None`` -- an externally-scored run (``SKIP_ENDPOINT_PHASE``) whose + merge gate refused, a scorer whose responses never arrived. Without this + check the process exits 0, ``report.txt`` prints ``N/A``, and every wrapper + downstream (sbatch disposition, CI gate, dashboard) reads the run as a pass. + That failure shape -- rc=0, all work "done", no number -- is the one that + costs whole GPU allocations, so it is made loud here rather than left to + each caller to notice. + + Scored-but-partial (a real number with ``complete=False``) is not failed: + the number exists and the entry already says it is partial. + """ + # A PERF-mode run skips externally-scored datasets entirely (they are never + # dispatched), so they are not owed a number here. + expected = { + ec.dataset_name + for ec in ctx.eval_configs + if ec.dataset_type == DatasetType.ACCURACY + and not (ctx.test_mode == TestMode.PERF and ec.scorer.SKIP_ENDPOINT_PHASE) + } + if not expected: + return + + scored = { + entry["dataset_name"] + for entry in accuracy_scores + if entry.get("dataset_type") == DatasetType.ACCURACY.value + and entry.get("score") is not None + } + missing = sorted(expected - scored) + if missing: + raise ExecutionError( + "accuracy scoring produced no score for: " + + ", ".join(missing) + + f". Artifacts were preserved in {ctx.report_dir}; see " + "accuracy/accuracy_results.json for the per-dataset detail." + ) + def run_benchmark( config: BenchmarkConfig, diff --git a/tests/unit/commands/test_benchmark.py b/tests/unit/commands/test_benchmark.py index a5ad0e6a4..82f78052a 100644 --- a/tests/unit/commands/test_benchmark.py +++ b/tests/unit/commands/test_benchmark.py @@ -155,6 +155,45 @@ def score_single_sample(self, value, ground_truth): return 0.0 +class _ScorelessExternalScorer(Scorer, scorer_id="_test_scoreless_external"): + """Does all its work, exits cleanly, and returns no number. + + Models the real shape of the SWE-bench fleet scorer whose merge gate + refuses: every unit reached a terminal record, nothing raised, and the + headline is ``None``. + """ + + SKIP_ENDPOINT_PHASE = True + + @classmethod + def external_sample_count(cls, extras): + return 2 + + def score_single_sample(self, value, ground_truth): + return 0.0 + + def score(self): + self.complete = False + return None, 1 + + +class _PartialButScoredScorer(Scorer, scorer_id="_test_partial_but_scored"): + """A real number flagged partial — reported, not failed.""" + + SKIP_ENDPOINT_PHASE = True + + @classmethod + def external_sample_count(cls, extras): + return 2 + + def score_single_sample(self, value, ground_truth): + return 0.0 + + def score(self): + self.complete = False + return 0.25, 1 + + class _OrdinaryAccuracyScorer(Scorer, scorer_id="_test_ordinary_accuracy"): def score_single_sample(self, value, ground_truth): return 0.0 @@ -2563,6 +2602,129 @@ def _make_report(state: str) -> Report: ) +class TestAccuracyRunWithoutANumberFails: + """rc=0, all work done, no number is a FAILURE. + + This is the regression that cost a whole 200-instance GPU run: the + distributed SWE-bench scorer drove all 20 units to terminal records, the + merge gate refused (17 abandoned), ``score()`` returned ``None``, + ``report.txt`` printed ``N/A`` — and the process exited 0, so the sbatch + wrapper wrote ``disposition=run completed``. Any scorer path that finishes + its units but produces no accuracy must fail loudly instead. + """ + + def _eval_config(self, scorer, dataset, report_dir: Path, name: str): + return AccuracyConfiguration( + scorer=scorer, + extractor=None, + dataset_name=name, + dataset=dataset, + report_dir=report_dir, + ground_truth_column=None, + num_repeats=1, + dataset_type=DatasetType.ACCURACY, + ) + + def _ctx(self, tmp_path, scorer, name="external_accuracy"): + dataset = _make_loaded_dataset() + return _make_benchmark_context( + config=OfflineConfig(**_OFFLINE_KWARGS), + report_dir=tmp_path, + test_mode=TestMode.ACC, + dataloader=dataset, + eval_configs=[self._eval_config(scorer, dataset, tmp_path, name)], + ) + + @pytest.mark.unit + def test_scoreless_accuracy_run_raises(self, tmp_path): + ctx = self._ctx(tmp_path, _ScorelessExternalScorer) + + with pytest.raises(ExecutionError, match="produced no score"): + finalize_benchmark(ctx, _make_benchmark_result(tmp_path)) + + @pytest.mark.unit + def test_artifacts_survive_the_failure(self, tmp_path): + """The failure must not cost the evidence needed to diagnose it.""" + ctx = self._ctx(tmp_path, _ScorelessExternalScorer) + + with pytest.raises(ExecutionError): + finalize_benchmark(ctx, _make_benchmark_result(tmp_path)) + + results = json.loads( + (tmp_path / "accuracy" / "accuracy_results.json").read_text() + ) + entry = results["accuracy_scores"][0] + assert entry["dataset_name"] == "external_accuracy" + assert entry["score"] is None + assert entry["complete"] is False + + @pytest.mark.unit + def test_one_scored_dataset_does_not_excuse_a_scoreless_peer(self, tmp_path): + """Averaging over datasets must not hide a dataset that scored nothing.""" + dataset = _make_loaded_dataset() + ctx = _make_benchmark_context( + config=OfflineConfig(**_OFFLINE_KWARGS), + report_dir=tmp_path, + test_mode=TestMode.ACC, + dataloader=dataset, + eval_configs=[ + self._eval_config(_SelfContainedScorer, dataset, tmp_path, "good"), + self._eval_config(_ScorelessExternalScorer, dataset, tmp_path, "bad"), + ], + ) + + with pytest.raises(ExecutionError, match="bad"): + finalize_benchmark(ctx, _make_benchmark_result(tmp_path)) + + @pytest.mark.unit + def test_partial_but_numeric_score_still_passes(self, tmp_path): + """A real number flagged incomplete is reportable, not a failure.""" + ctx = self._ctx(tmp_path, _PartialButScoredScorer) + + finalize_benchmark(ctx, _make_benchmark_result(tmp_path)) + + results = json.loads( + (tmp_path / "accuracy" / "accuracy_results.json").read_text() + ) + assert results["accuracy_scores"][0]["score"] == 0.25 + assert results["accuracy_scores"][0]["complete"] is False + + @pytest.mark.unit + def test_perf_only_run_owes_no_accuracy_number(self, tmp_path): + """PERF mode never dispatches an external scorer, so it owes nothing.""" + dataset = _make_loaded_dataset() + ctx = _make_benchmark_context( + config=OfflineConfig(**_OFFLINE_KWARGS), + report_dir=tmp_path, + test_mode=TestMode.PERF, + dataloader=dataset, + eval_configs=[ + self._eval_config( + _ScorelessExternalScorer, dataset, tmp_path, "external_accuracy" + ) + ], + ) + + finalize_benchmark(ctx, _make_benchmark_result(tmp_path)) + + @pytest.mark.unit + def test_external_scorer_sample_count_is_the_evaluated_count( + self, tmp_path, caplog + ): + """'N samples evaluated' must match the dataset's own unit= line. + + SWE-bench Verified loads 500 rows and scores ``num_instances`` of them; + reporting 500 against a ``unit=200`` headline is how a wrong scope goes + unnoticed. + """ + ctx = self._ctx(tmp_path, _PartialButScoredScorer) + + with caplog.at_level(logging.INFO, logger=execute_mod.logger.name): + finalize_benchmark(ctx, _make_benchmark_result(tmp_path)) + + assert "Accuracy-only: 2 samples evaluated" in caplog.text + + class TestScorerMethodSync: """Ensure ScorerMethod enum stays in sync with the scorer registry."""