Copilot commented on code in PR #128: URL: https://github.com/apache/sedona-spatialbench/pull/128#discussion_r3636313160
########## benchmark/verify_results.py: ########## @@ -0,0 +1,277 @@ +#!/usr/bin/env python3 +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +""" +Verify SpatialBench correctness by comparing the benchmark run's result dumps +against the committed ground-truth answers, and render a summary table. + +This runs as a downstream CI job that *needs* the benchmark jobs and reuses their +output: the benchmark run writes each query's normalized result to +``<engine>_<query>_result.csv`` (see run_benchmark.py --result-dir); this script +reads those dumps plus the committed answers in ``benchmark/answers/sf<sf>`` and +compares them. It is engine-free (only pandas/numpy), so none of the SedonaDB / +pyarrow import-order pitfalls apply here. + +Comparison rules (same as the answers were blessed with): + * columns compared by position (engines name e.g. avg_duration differently) + * integer keys / strings / timestamps: exact + * floats (distances, areas, IoU, seconds): rtol=1e-6, atol=1e-9 + * a within-tolerance difference confined to the final LIMIT-boundary row passes + +Exit status: non-zero if any engine's result MISMATCHES its committed answer, so a +regression fails CI. An engine that could not produce a result (timeout / error / +OOM, e.g. DuckDB's lateral-join Q12 at scale) is reported but does not fail the +job — that is a runtime issue surfaced by the benchmark summary, not a wrong answer. +""" + +import argparse +import json +import sys +from datetime import datetime, timezone +from pathlib import Path + +import numpy as np +import pandas as pd + +QUERY_COUNT = 12 +ENGINES = ["duckdb", "geopandas", "sedonadb", "spatial_polars", "pycanopy"] +ENGINE_ICONS = { + "sedonadb": "🌵 SedonaDB", + "duckdb": "🦆 DuckDB", + "geopandas": "🐼 GeoPandas", + "spatial_polars": "🐻❄️ Spatial Polars", + "pycanopy": "🌴 PyCanopy", +} + + +def compare(answer: pd.DataFrame, result: pd.DataFrame, rtol: float, atol: float) -> list[str]: + """Positional comparison of a result against the answer.""" + if answer.shape[1] != result.shape[1]: + return [f"column count differs: answer {list(answer.columns)} vs engine {list(result.columns)}"] + issues = [] + if len(answer) != len(result): + issues.append(f"row count differs: answer {len(answer)} vs engine {len(result)}") + n = min(len(answer), len(result)) + for i in range(answer.shape[1]): + name = answer.columns[i] + a = answer.iloc[:n, i].reset_index(drop=True) + b = result.iloc[:n, i].reset_index(drop=True) + if pd.api.types.is_float_dtype(a) or pd.api.types.is_float_dtype(b): + va = pd.to_numeric(a, errors="coerce").to_numpy(dtype=float) + vb = pd.to_numeric(b, errors="coerce").to_numpy(dtype=float) + close = np.isclose(va, vb, rtol=rtol, atol=atol, equal_nan=True) + bad = int((~close).sum()) + if bad: + j = int(np.where(~close)[0][0]) + issues.append( + f"col {i} ('{name}'): {bad}/{n} float mismatch " + f"(first at row {j}: {va[j]!r} vs {vb[j]!r})" + ) + else: + neq = a.astype("string").fillna("<NA>") != b.astype("string").fillna("<NA>") + bad = int(neq.sum()) + if bad: + j = int(np.where(neq.to_numpy())[0][0]) + issues.append( + f"col {i} ('{name}'): {bad}/{n} mismatch " + f"(first at row {j}: {a.iloc[j]!r} vs {b.iloc[j]!r})" + ) + return issues + + +def boundary_only(issues: list[str], answer: pd.DataFrame) -> bool: + """True if every discrepancy is confined to the final row (LIMIT-boundary tie).""" + if not issues: + return False + last = len(answer) - 1 + return all(f"row {last}:" in msg or f"at row {last}" in msg for msg in issues) + + +def load_statuses(results_dir: Path) -> dict: + """Map (engine, query) -> benchmark run status, from the timing JSONs. + + Lets the report distinguish a timeout/OOM (couldn't run) from a plain missing + dump, without treating either as a correctness failure. + """ + statuses = {} + for jf in results_dir.glob("*_results.json"): + try: + data = json.loads(jf.read_text()) + except Exception: + continue + for suite in data.get("results", []): + engine = suite.get("engine") + for r in suite.get("results", []): + statuses[(engine, r.get("query"))] = r.get("status") + return statuses + + +def verify_one(results_dir: Path, answers_dir: Path, engine: str, query: str, + status: str | None, rtol: float, atol: float) -> tuple[str, str | None]: + """Return (verdict, detail). verdict is one of: + pass, fail, no_answer, timeout, oom, run_error, no_result. + Only 'fail' counts as a correctness failure. + """ + answer_csv = answers_dir / f"{query}.csv" + result_csv = results_dir / f"{engine}_{query}_result.csv" + if not answer_csv.exists(): + return "no_answer", None + if not result_csv.exists(): + if status == "timeout": + return "timeout", None + if status == "not_started": + return "oom", None + if status == "error": + return "run_error", None + return "no_result", None Review Comment: If the benchmark JSON reports `status == "success"` but the corresponding `_<engine>_<query>_result.csv` is missing, the job currently returns `no_result` and does not fail CI. That weakens the correctness gate because it can silently skip verification due to dump/upload bugs. Treat a missing dump for a successful run as a failure (or introduce a dedicated failing verdict). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
