From 0c06dac9762ab5097dae6ce773b5a07a6766a281 Mon Sep 17 00:00:00 2001 From: Abimael Martell <1450169+abimaelmartell@users.noreply.github.com> Date: Thu, 16 Jul 2026 14:25:25 -0700 Subject: [PATCH] test(bench): compare OpenDataLoader builds (#175) * test(bench): compare OpenDataLoader builds * docs(bench): keep reference comparisons generic * fix(bench): keep regression gates complete * fix(bench): clarify missing reference gates * fix(bench): validate nonnegative limits * fix(bench): isolate prediction runs * chore(bench): refresh review --- .github/workflows/ci.yml | 3 + README.md | 14 +- docs/benchmarking.md | 29 ++ docs/python.md | 4 +- docs/rust-api.md | 4 +- scripts/bench_opendataloader.py | 351 +++++++++++++++++++++ scripts/tests/test_bench_opendataloader.py | 203 ++++++++++++ 7 files changed, 598 insertions(+), 10 deletions(-) create mode 100644 docs/benchmarking.md create mode 100644 scripts/bench_opendataloader.py create mode 100644 scripts/tests/test_bench_opendataloader.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 84590db..e3ae483 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -25,6 +25,9 @@ jobs: - name: Run tests run: cargo test --verbose + - name: Test benchmark harness + run: python3 -m unittest discover -s scripts/tests + fmt: name: Format runs-on: ubuntu-latest diff --git a/README.md b/README.md index 1fcc743..4617985 100644 --- a/README.md +++ b/README.md @@ -27,16 +27,18 @@ Evaluated on the [opendataloader-bench](https://github.com/opendataloader-projec | Engine | Overall | Reading Order (NID) | Tables (TEDS) | Headings (MHS) | Speed (200 docs) | |---|---|---|---|---|---| -| pdf-inspector | 0.83 | 0.89 | 0.66 | 0.74 | 4s | -| opendataloader | 0.84 | 0.91 | 0.49 | 0.74 | 11s | +| pdf-inspector | **0.875** | **0.915** | **0.814** | 0.788 | 3.3s | +| opendataloader | 0.831 | 0.902 | 0.489 | 0.739 | 3.0s | | pymupdf4llm | 0.73 | 0.89 | 0.40 | 0.41 | 18s | -| markitdown | 0.58 | 0.88 | 0.00 | 0.00 | 8s | +| markitdown | 0.59 | 0.84 | 0.27 | 0.00 | 23s | -For context, engines that use OCR/ML (docling, marker, mineru) score 0.83-0.88 overall but take 2-180 minutes on the same corpus — pdf-inspector reaches the low end of that range without any OCR, in 4 seconds. +For context, engines that use OCR/ML (docling, marker, mineru) score 0.83-0.88 overall but take 2-180 minutes on the same corpus — pdf-inspector reaches the top of that range without any OCR, in 3.3 seconds. -**Where we do well:** Speed (fastest of all engines), the best table detection of any engine shown, and heading detection now on par with opendataloader. Overall lands within 0.01 of opendataloader at roughly 2.5× the speed. +**Where we do well:** The best overall, reading-order, and table scores among the direct extraction engines shown. -**Where we lag:** Reading order still trails opendataloader slightly, and table structure trails OCR-based engines that can see visual layout. +**Where we lag:** Some direct engines remain slightly faster, and OCR-based engines can recover text that has no usable PDF text layer. + +Use the [paired benchmark harness](docs/benchmarking.md) to compare two local builds against the exact same corpus and evaluator revision. ## Quick start diff --git a/docs/benchmarking.md b/docs/benchmarking.md new file mode 100644 index 0000000..adbfda7 --- /dev/null +++ b/docs/benchmarking.md @@ -0,0 +1,29 @@ +# Benchmarking against OpenDataLoader + +The paired harness runs two `pdf2md` binaries through the same local +OpenDataLoader corpus, evaluates both outputs, and reports aggregate and +per-document deltas. This avoids comparing results produced from different +corpus revisions or evaluator versions. + +Build a candidate and provide a released or worktree build as the baseline: + +```bash +cargo build --release +python3 scripts/bench_opendataloader.py \ + --bench-dir ../opendataloader-bench \ + --baseline ../pdf-inspector-main/target/release/pdf2md \ + --candidate target/release/pdf2md \ + --max-document-regression 0.02 \ + --json-output /tmp/pdf-inspector-benchmark.json +``` + +Pass `--reference-evaluation path/to/evaluation.json` to report the candidate +delta against another evaluation, and add `--require-reference-lead` to make a +negative reference delta fail the run. By default, the candidate must not +regress the baseline overall score or introduce missing predictions. Use +`--min-overall-delta` to require a specific aggregate gain. + +The OpenDataLoader repository is external and keeps its normal +`prediction/pdf-inspector` output. Paired evaluation copies each run into a +temporary directory before evaluating it, so the baseline and candidate cannot +overwrite one another. diff --git a/docs/python.md b/docs/python.md index 88f7444..4a6cf77 100644 --- a/docs/python.md +++ b/docs/python.md @@ -18,8 +18,8 @@ Built by [Firecrawl](https://firecrawl.dev) to handle text-based PDFs locally in | Engine | Overall | Reading order | Tables (TEDS) | Headings | Speed | |---|---|---|---|---|---| -| **pdf-inspector** | 0.83 | 0.88 | **0.66** | 0.74 | **4s** | -| opendataloader | 0.84 | 0.91 | 0.49 | 0.74 | 11s | +| **pdf-inspector** | **0.875** | **0.915** | **0.814** | 0.788 | 3.3s | +| opendataloader | 0.831 | 0.902 | 0.489 | 0.739 | 3.0s | | pymupdf4llm | 0.73 | 0.89 | 0.40 | 0.41 | 18s | OCR/ML engines (docling, marker, mineru) score 0.83–0.88 overall but take 2–180 minutes on the same corpus. Full numbers in the [repo README](https://github.com/firecrawl/pdf-inspector#benchmark). diff --git a/docs/rust-api.md b/docs/rust-api.md index fc443a6..a80133a 100644 --- a/docs/rust-api.md +++ b/docs/rust-api.md @@ -18,8 +18,8 @@ Built by [Firecrawl](https://firecrawl.dev) to handle text-based PDFs locally in | Engine | Overall | Reading order | Tables (TEDS) | Headings | Speed | |---|---|---|---|---|---| -| **pdf-inspector** | 0.83 | 0.88 | **0.66** | 0.74 | **4s** | -| opendataloader | 0.84 | 0.91 | 0.49 | 0.74 | 11s | +| **pdf-inspector** | **0.875** | **0.915** | **0.814** | 0.788 | 3.3s | +| opendataloader | 0.831 | 0.902 | 0.489 | 0.739 | 3.0s | | pymupdf4llm | 0.73 | 0.89 | 0.40 | 0.41 | 18s | OCR/ML engines (docling, marker, mineru) score 0.83–0.88 overall but take 2–180 minutes on the same corpus. Full numbers in the [repo README](https://github.com/firecrawl/pdf-inspector#benchmark). diff --git a/scripts/bench_opendataloader.py b/scripts/bench_opendataloader.py new file mode 100644 index 0000000..1c0a8ec --- /dev/null +++ b/scripts/bench_opendataloader.py @@ -0,0 +1,351 @@ +#!/usr/bin/env python3 +"""Run a paired pdf-inspector OpenDataLoader benchmark and report deltas.""" + +from __future__ import annotations + +import argparse +import json +import math +import os +import shutil +import subprocess +import sys +import tempfile +from pathlib import Path +from typing import Any + + +SCORE_KEYS = ( + "overall_mean", + "nid_mean", + "nid_s_mean", + "teds_mean", + "teds_s_mean", + "mhs_mean", + "mhs_s_mean", +) + + +def _non_negative_int(value: str) -> int: + parsed = int(value) + if parsed < 0: + raise argparse.ArgumentTypeError("must be non-negative") + return parsed + + +def _non_negative_float(value: str) -> float: + parsed = float(value) + if not math.isfinite(parsed) or parsed < 0.0: + raise argparse.ArgumentTypeError("must be finite and non-negative") + return parsed + + +def _finite_float(value: str) -> float: + parsed = float(value) + if not math.isfinite(parsed): + raise argparse.ArgumentTypeError("must be finite") + return parsed + + +def _scores(evaluation: dict[str, Any]) -> dict[str, float]: + score = evaluation.get("metrics", {}).get("score", {}) + return {key: float(score[key]) for key in SCORE_KEYS if score.get(key) is not None} + + +def _documents(evaluation: dict[str, Any]) -> dict[str, float]: + documents: dict[str, float] = {} + for document in evaluation.get("documents", []): + overall = document.get("scores", {}).get("overall") + if overall is not None: + documents[str(document["document_id"])] = float(overall) + return documents + + +def compare_evaluations( + baseline: dict[str, Any], + candidate: dict[str, Any], + reference: dict[str, Any] | None = None, + *, + top: int = 10, +) -> dict[str, Any]: + """Build aggregate and per-document deltas from evaluator JSON payloads.""" + baseline_scores = _scores(baseline) + candidate_scores = _scores(candidate) + metric_deltas = { + key: candidate_scores[key] - baseline_scores[key] + for key in SCORE_KEYS + if key in baseline_scores and key in candidate_scores + } + + baseline_documents = _documents(baseline) + candidate_documents = _documents(candidate) + shared = sorted(baseline_documents.keys() & candidate_documents.keys()) + document_deltas = [ + { + "document_id": document_id, + "baseline": baseline_documents[document_id], + "candidate": candidate_documents[document_id], + "delta": candidate_documents[document_id] - baseline_documents[document_id], + } + for document_id in shared + ] + epsilon = 1e-12 + improvements = sorted(document_deltas, key=lambda item: item["delta"], reverse=True) + regressions = sorted(document_deltas, key=lambda item: item["delta"]) + + result: dict[str, Any] = { + "baseline": baseline_scores, + "candidate": candidate_scores, + "deltas": metric_deltas, + "missing_predictions": { + "baseline": int(baseline.get("metrics", {}).get("missing_predictions", 0)), + "candidate": int(candidate.get("metrics", {}).get("missing_predictions", 0)), + }, + "documents": { + "shared": len(shared), + "improved": sum(item["delta"] > epsilon for item in document_deltas), + "regressed": sum(item["delta"] < -epsilon for item in document_deltas), + "unchanged": sum(abs(item["delta"]) <= epsilon for item in document_deltas), + "largest_improvements": [ + item for item in improvements if item["delta"] > epsilon + ][:top], + "largest_regressions": [ + item for item in regressions if item["delta"] < -epsilon + ][:top], + "worst_regression": next( + (item for item in regressions if item["delta"] < -epsilon), None + ), + }, + } + if reference is not None: + reference_scores = _scores(reference) + result["reference"] = reference_scores + result["candidate_vs_reference"] = { + key: candidate_scores[key] - reference_scores[key] + for key in SCORE_KEYS + if key in candidate_scores and key in reference_scores + } + return result + + +def evaluate_gates( + comparison: dict[str, Any], + *, + min_overall_delta: float, + max_document_regression: float | None, + max_missing: int, + require_reference_lead: bool, +) -> list[str]: + """Return human-readable gate failures; an empty list means pass.""" + failures: list[str] = [] + overall_delta = comparison["deltas"].get("overall_mean") + if overall_delta is None or overall_delta < min_overall_delta: + failures.append( + f"overall delta {overall_delta!r} is below {min_overall_delta:+.6f}" + ) + candidate_missing = comparison["missing_predictions"]["candidate"] + if candidate_missing > max_missing: + failures.append( + f"candidate has {candidate_missing} missing predictions (maximum {max_missing})" + ) + if max_document_regression is not None: + regression = comparison["documents"].get("worst_regression") + if regression is not None and regression["delta"] < -max_document_regression: + failures.append( + "largest document regression " + f"{regression['document_id']}={regression['delta']:+.6f} " + f"exceeds {-max_document_regression:+.6f}" + ) + if require_reference_lead: + reference_delta = comparison.get("candidate_vs_reference", {}).get("overall_mean") + if reference_delta is None: + failures.append("reference overall score is unavailable") + elif reference_delta < 0.0: + failures.append( + f"candidate trails reference overall by {reference_delta!r}" + ) + return failures + + +def _run(command: list[str], *, cwd: Path, env: dict[str, str] | None = None) -> None: + print("+", " ".join(command), flush=True) + subprocess.run(command, cwd=cwd, env=env, check=True) + + +def _run_engine( + *, + bench_dir: Path, + python: Path, + binary: Path, + label: str, + scratch_root: Path, +) -> dict[str, Any]: + env = os.environ.copy() + env["PDF_INSPECTOR_BINARY"] = str(binary) + source = bench_dir / "prediction" / "pdf-inspector" + if source.exists(): + if source.is_dir(): + shutil.rmtree(source) + else: + source.unlink() + _run( + [ + str(python), + "src/pdf_parser.py", + "--engine", + "pdf-inspector", + "--log-level", + "WARNING", + ], + cwd=bench_dir, + env=env, + ) + + if not source.is_dir(): + raise RuntimeError(f"parser did not produce predictions: {source}") + destination = scratch_root / label + shutil.copytree(source, destination) + _run( + [ + str(python), + "src/evaluator.py", + "--prediction-root", + str(scratch_root), + "--engine", + label, + "--log-level", + "WARNING", + ], + cwd=bench_dir, + ) + with (destination / "evaluation.json").open(encoding="utf-8") as handle: + return json.load(handle) + + +def _print_report(comparison: dict[str, Any]) -> None: + print("\nMetric baseline candidate delta") + print("-------------------- ---------- ---------- ----------") + for key in SCORE_KEYS: + if key not in comparison["deltas"]: + continue + print( + f"{key:<20} {comparison['baseline'][key]:>10.6f} " + f"{comparison['candidate'][key]:>10.6f} " + f"{comparison['deltas'][key]:>+10.6f}" + ) + if "reference" in comparison: + delta = comparison["candidate_vs_reference"].get("overall_mean") + reference = comparison["reference"].get("overall_mean") + reference_display = f"{reference:.6f}" if reference is not None else "n/a" + delta_display = f"{delta:+.6f}" if delta is not None else "n/a" + print(f"\nReference overall: {reference_display}; candidate delta: {delta_display}") + + documents = comparison["documents"] + print( + "\nDocuments: " + f"{documents['improved']} improved, {documents['regressed']} regressed, " + f"{documents['unchanged']} unchanged ({documents['shared']} shared)" + ) + for heading, key in ( + ("Largest improvements", "largest_improvements"), + ("Largest regressions", "largest_regressions"), + ): + print(f"\n{heading}:") + rows = documents[key] + if not rows: + print(" none") + for row in rows: + print( + f" {row['document_id']}: {row['delta']:+.6f} " + f"({row['baseline']:.6f} -> {row['candidate']:.6f})" + ) + + +def _arguments(argv: list[str] | None = None) -> argparse.Namespace: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--bench-dir", type=Path, required=True) + parser.add_argument("--baseline", type=Path, required=True) + parser.add_argument("--candidate", type=Path, required=True) + parser.add_argument("--python", type=Path) + parser.add_argument("--reference-evaluation", type=Path) + parser.add_argument("--json-output", type=Path) + parser.add_argument("--top", type=_non_negative_int, default=10) + parser.add_argument("--min-overall-delta", type=_finite_float, default=0.0) + parser.add_argument("--max-document-regression", type=_non_negative_float) + parser.add_argument("--max-missing", type=_non_negative_int, default=0) + parser.add_argument("--require-reference-lead", action="store_true") + return parser.parse_args(argv) + + +def main(argv: list[str] | None = None) -> int: + args = _arguments(argv) + bench_dir = args.bench_dir.resolve() + baseline = args.baseline.resolve() + candidate = args.candidate.resolve() + # Keep the virtualenv launcher path intact. Resolving its symlink would + # invoke the underlying system interpreter without the benchmark's site + # packages. + python = (args.python or bench_dir / ".venv" / "bin" / "python").absolute() + for path, description in ( + (bench_dir / "src" / "pdf_parser.py", "OpenDataLoader parser"), + (bench_dir / "src" / "evaluator.py", "OpenDataLoader evaluator"), + (baseline, "baseline binary"), + (candidate, "candidate binary"), + (python, "Python interpreter"), + ): + if not path.exists(): + raise SystemExit(f"{description} not found: {path}") + + with tempfile.TemporaryDirectory(prefix="pdf-inspector-opendataloader-") as temporary: + scratch_root = Path(temporary) + baseline_evaluation = _run_engine( + bench_dir=bench_dir, + python=python, + binary=baseline, + label="baseline", + scratch_root=scratch_root, + ) + candidate_evaluation = _run_engine( + bench_dir=bench_dir, + python=python, + binary=candidate, + label="candidate", + scratch_root=scratch_root, + ) + + reference = None + if args.reference_evaluation is not None: + with args.reference_evaluation.resolve().open(encoding="utf-8") as handle: + reference = json.load(handle) + + comparison = compare_evaluations( + baseline_evaluation, + candidate_evaluation, + reference, + top=args.top, + ) + + _print_report(comparison) + if args.json_output is not None: + args.json_output.resolve().write_text( + json.dumps(comparison, indent=2) + "\n", encoding="utf-8" + ) + + failures = evaluate_gates( + comparison, + min_overall_delta=args.min_overall_delta, + max_document_regression=args.max_document_regression, + max_missing=args.max_missing, + require_reference_lead=args.require_reference_lead, + ) + if failures: + print("\nBenchmark gate failed:", file=sys.stderr) + for failure in failures: + print(f" - {failure}", file=sys.stderr) + return 1 + print("\nBenchmark gate passed.") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/tests/test_bench_opendataloader.py b/scripts/tests/test_bench_opendataloader.py new file mode 100644 index 0000000..d789e64 --- /dev/null +++ b/scripts/tests/test_bench_opendataloader.py @@ -0,0 +1,203 @@ +import io +import json +import sys +import tempfile +import unittest +from contextlib import redirect_stderr, redirect_stdout +from pathlib import Path +from unittest.mock import patch + +sys.path.insert(0, str(Path(__file__).resolve().parents[1])) + +from bench_opendataloader import ( + _arguments, + _print_report, + _run_engine, + compare_evaluations, + evaluate_gates, +) + + +def evaluation(overall, documents, *, missing=0): + return { + "metrics": { + "score": { + "overall_mean": overall, + "nid_mean": overall + 0.01, + }, + "missing_predictions": missing, + }, + "documents": [ + { + "document_id": document_id, + "scores": {"overall": score}, + } + for document_id, score in documents.items() + ], + } + + +class ComparisonTests(unittest.TestCase): + def test_reports_metric_and_document_deltas(self): + baseline = evaluation(0.80, {"a": 0.8, "b": 0.6, "c": 0.7}) + candidate = evaluation(0.82, {"a": 0.9, "b": 0.5, "c": 0.7}) + + result = compare_evaluations(baseline, candidate, top=1) + + self.assertAlmostEqual(result["deltas"]["overall_mean"], 0.02) + self.assertEqual(result["documents"]["improved"], 1) + self.assertEqual(result["documents"]["regressed"], 1) + self.assertEqual(result["documents"]["unchanged"], 1) + self.assertEqual( + result["documents"]["largest_improvements"][0]["document_id"], "a" + ) + self.assertEqual( + result["documents"]["largest_regressions"][0]["document_id"], "b" + ) + + def test_reference_delta_is_reported(self): + baseline = evaluation(0.80, {}) + candidate = evaluation(0.82, {}) + reference = evaluation(0.81, {}) + + result = compare_evaluations(baseline, candidate, reference) + + self.assertAlmostEqual( + result["candidate_vs_reference"]["overall_mean"], 0.01 + ) + + def test_gates_cover_aggregate_document_missing_and_reference(self): + comparison = compare_evaluations( + evaluation(0.80, {"a": 0.8}), + evaluation(0.79, {"a": 0.7}, missing=1), + evaluation(0.81, {}), + ) + + failures = evaluate_gates( + comparison, + min_overall_delta=0.0, + max_document_regression=0.05, + max_missing=0, + require_reference_lead=True, + ) + + self.assertEqual(len(failures), 4) + + def test_regression_gate_is_independent_of_report_limit(self): + comparison = compare_evaluations( + evaluation(0.80, {"a": 0.8}), + evaluation(0.80, {"a": 0.7}), + top=0, + ) + + failures = evaluate_gates( + comparison, + min_overall_delta=0.0, + max_document_regression=0.05, + max_missing=0, + require_reference_lead=False, + ) + + self.assertEqual(len(failures), 1) + self.assertIn("largest document regression", failures[0]) + + def test_report_handles_reference_without_overall_score(self): + result = compare_evaluations( + evaluation(0.80, {}), + evaluation(0.82, {}), + {"metrics": {"score": {"nid_mean": 0.81}}}, + ) + + output = io.StringIO() + with redirect_stdout(output): + _print_report(result) + + self.assertIn("Reference overall: n/a; candidate delta: n/a", output.getvalue()) + + def test_reference_gate_reports_missing_score_as_unavailable(self): + comparison = compare_evaluations( + evaluation(0.80, {}), + evaluation(0.82, {}), + ) + + failures = evaluate_gates( + comparison, + min_overall_delta=0.0, + max_document_regression=None, + max_missing=0, + require_reference_lead=True, + ) + + self.assertEqual(failures, ["reference overall score is unavailable"]) + + def test_arguments_reject_negative_counts_and_allow_zero_top(self): + required = [ + "--bench-dir", + ".", + "--baseline", + "baseline", + "--candidate", + "candidate", + ] + self.assertEqual(_arguments(required + ["--top", "0"]).top, 0) + for option in ("--top", "--max-document-regression", "--max-missing"): + with self.subTest(option=option), redirect_stderr(io.StringIO()): + with self.assertRaises(SystemExit): + _arguments(required + [option, "-1"]) + + def test_arguments_reject_nonfinite_float_thresholds(self): + required = [ + "--bench-dir", + ".", + "--baseline", + "baseline", + "--candidate", + "candidate", + ] + for option in ("--min-overall-delta", "--max-document-regression"): + for value in ("nan", "inf", "-inf"): + with self.subTest(option=option, value=value), redirect_stderr( + io.StringIO() + ): + with self.assertRaises(SystemExit): + _arguments(required + [option, value]) + + def test_run_engine_clears_stale_predictions_before_parser(self): + with tempfile.TemporaryDirectory() as temporary: + root = Path(temporary) + bench_dir = root / "bench" + source = bench_dir / "prediction" / "pdf-inspector" + source.mkdir(parents=True) + (source / "stale.md").write_text("stale", encoding="utf-8") + scratch = root / "scratch" + scratch.mkdir() + + def fake_run(command, *, cwd, env=None): + if any(part.endswith("pdf_parser.py") for part in command): + self.assertFalse(source.exists()) + (source / "markdown").mkdir(parents=True) + (source / "markdown" / "new.md").write_text( + "new", encoding="utf-8" + ) + else: + destination = scratch / "candidate" + (destination / "evaluation.json").write_text( + json.dumps(evaluation(0.82, {})), encoding="utf-8" + ) + + with patch("bench_opendataloader._run", side_effect=fake_run): + result = _run_engine( + bench_dir=bench_dir, + python=Path("python"), + binary=Path("pdf2md"), + label="candidate", + scratch_root=scratch, + ) + + self.assertEqual(result["metrics"]["score"]["overall_mean"], 0.82) + self.assertFalse((source / "stale.md").exists()) + self.assertFalse((scratch / "candidate" / "stale.md").exists()) + + +if __name__ == "__main__": + unittest.main()