From 5470c2d686069f208ef9ca38e6709238955a04b4 Mon Sep 17 00:00:00 2001 From: dxnnystyles Date: Thu, 29 Oct 2026 07:39:04 -0700 Subject: [PATCH] feat: merge selected audits into benchmark results --- README.md | 4 + faircode/benchmark.py | 85 +++++++++++++++++- faircode/cli.py | 5 +- tests/test_results_provenance.py | 145 +++++++++++++++++++++++++++++++ 4 files changed, 234 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index a7a14f55..257fb4ba 100644 --- a/README.md +++ b/README.md @@ -1289,6 +1289,7 @@ faircode benchmark COMPAS/audit.yaml # run a subset explicitly faircode benchmark --root ~/audits # search ~/audits/*/audit.yaml instead of the current directory faircode benchmark --no-plots # CSVs only: no figures/ and matplotlib is not needed faircode benchmark --quiet # suppress per-combination progress on stderr +faircode benchmark --merge COMPAS/audit.yaml # replace selected audits in an existing verified result set ``` `--root PATH` is the directory searched for `*/audit.yaml` (one level down; default: the current @@ -1297,6 +1298,9 @@ paths. `--no-plots` skips the `figures/*.png` files; `results_fairness.csv`, `re and `summary.csv` are still written. By default, progress is written to stderr after each audit/strategy/model combination completes. `--quiet` suppresses those progress lines; the existing final summary and warnings remain on stderr. +`--merge` replaces rows only for the audits passed to that run, preserves other audits and prior run records, +and refreshes the provenance hashes. It requires a complete existing output with valid `PROVENANCE.json`; +legacy or inconsistent output is left untouched and rejected. Writes `results_fairness.csv`, `results_performance.csv`, `summary.csv`, and one `figures/_strategies.png` per audit (300 dpi, rendered by `faircode/figures.py` straight from diff --git a/faircode/benchmark.py b/faircode/benchmark.py index 0ab5512e..ca4a5d4d 100644 --- a/faircode/benchmark.py +++ b/faircode/benchmark.py @@ -33,8 +33,11 @@ import hashlib import itertools import json +import os import platform +import shutil import subprocess +import tempfile from dataclasses import dataclass from importlib import metadata from pathlib import Path @@ -411,7 +414,7 @@ def write_provenance(out_dir, runs) -> Path: return path -def _validate_existing_result_outputs(out_dir, requested_audits, provenance) -> None: +def _validate_existing_result_outputs(out_dir, requested_audits, provenance, *, allow_other_audits=False): """Validate existing results before report writes to prevent partial replacement.""" out_dir = Path(out_dir) paths = {name: out_dir / name for name in RESULT_FILES} @@ -480,7 +483,7 @@ def _validate_existing_result_outputs(out_dir, requested_audits, provenance) -> ) other_audits = existing_audits - requested_audits - if other_audits: + if other_audits and not allow_other_audits: names = ", ".join(sorted(other_audits)) raise ValueError( f"existing benchmark output contains results for other audits ({names}); " @@ -488,15 +491,89 @@ def _validate_existing_result_outputs(out_dir, requested_audits, provenance) -> ) +def _write_merged_report(fairness_df, performance_df, out_dir, make_plots, plot_metric, provenance): + """Stage a merged result set and roll back replaced files if installation fails.""" + out_dir = Path(out_dir) + old_fairness = pd.read_csv(out_dir / RESULT_FILES[0]) + old_performance = pd.read_csv(out_dir / RESULT_FILES[1]) + old_record = json.loads((out_dir / PROVENANCE_FILE).read_text(encoding="utf-8")) + if list(old_fairness.columns) != list(fairness_df.columns) or list(old_performance.columns) != list(performance_df.columns): + raise ValueError("--merge requires result tables with the existing column layout") + selected = set(fairness_df["audit"].astype(str)) + merged_fairness = pd.concat( + [old_fairness[~old_fairness["audit"].astype(str).isin(selected)], fairness_df], + ignore_index=True, + ) + merged_performance = pd.concat( + [old_performance[~old_performance["audit"].astype(str).isin(selected)], performance_df], + ignore_index=True, + ) + + with tempfile.TemporaryDirectory(prefix=".faircode-merge-", dir=out_dir.parent) as temporary: + stage = Path(temporary) / "stage" + stage.mkdir() + merged_fairness.to_csv(stage / RESULT_FILES[0], index=False) + merged_performance.to_csv(stage / RESULT_FILES[1], index=False) + summarize(merged_fairness).to_csv(stage / RESULT_FILES[2], index=False) + if make_plots and not merged_fairness.empty: + from .figures import generate_figures + generate_figures(stage, metric=plot_metric) + write_provenance(stage, [*old_record["runs"], provenance]) + + staged = [stage / name for name in RESULT_FILES] + if make_plots and (stage / "figures").is_dir(): + staged.extend(sorted((stage / "figures").glob("*.png"))) + staged.append(stage / PROVENANCE_FILE) + backup = Path(temporary) / "backup" + backup.mkdir() + installed = [] + saved = [] + try: + for source in staged: + relative = source.relative_to(stage) + destination = out_dir / relative + destination.parent.mkdir(parents=True, exist_ok=True) + if destination.exists(): + saved_path = backup / relative + saved_path.parent.mkdir(parents=True, exist_ok=True) + shutil.copy2(destination, saved_path) + saved.append((saved_path, destination)) + installed.append(destination) + os.replace(source, destination) + except BaseException: + for destination in reversed(installed): + destination.unlink(missing_ok=True) + for saved_path, destination in reversed(saved): + os.replace(saved_path, destination) + raise + return out_dir + + def write_report(fairness_df: pd.DataFrame, performance_df: pd.DataFrame, out_dir, - make_plots: bool = True, plot_metric: str = "demographic_parity_diff", provenance=None): + make_plots: bool = True, plot_metric: str = "demographic_parity_diff", provenance=None, + merge: bool = False): """Write results_fairness.csv, results_performance.csv, and a summary.csv to out_dir, plus (if make_plots) render figures/*.png via faircode.figures. - With `provenance` (a dict from collect_provenance) also write PROVENANCE.json (#1129).""" + With `provenance` (a dict from collect_provenance) also write PROVENANCE.json (#1129). + `merge=True` replaces only the requested audits in an existing verified result set.""" fairness_audits = set(fairness_df["audit"].astype(str)) if "audit" in fairness_df else set() performance_audits = set(performance_df["audit"].astype(str)) if "audit" in performance_df else set() if fairness_audits != performance_audits: raise ValueError("benchmark result tables contain different audit sets; refusing to write output") + if merge: + out_dir = Path(out_dir) + if not fairness_audits: + raise ValueError("--merge requires at least one audit") + if not isinstance(provenance, dict): + raise ValueError("--merge requires provenance for the new benchmark run") + run_audits = provenance.get("audits") + if not isinstance(run_audits, list) or set(map(str, run_audits)) != fairness_audits: + raise ValueError("--merge provenance audits do not match the benchmark result tables") + if not (out_dir / PROVENANCE_FILE).is_file(): + raise ValueError("--merge requires an existing, provenance-verified benchmark output") + _validate_existing_result_outputs(out_dir, fairness_audits, provenance, allow_other_audits=True) + return _write_merged_report(fairness_df, performance_df, out_dir, make_plots, plot_metric, provenance) + _validate_existing_result_outputs(out_dir, fairness_audits, provenance) out_dir = Path(out_dir) out_dir.mkdir(parents=True, exist_ok=True) diff --git a/faircode/cli.py b/faircode/cli.py index c894f54a..a693ff27 100644 --- a/faircode/cli.py +++ b/faircode/cli.py @@ -540,6 +540,8 @@ def build_parser() -> argparse.ArgumentParser: help="skip rendering figures/*.png (no matplotlib needed)") b.add_argument("--quiet", action="store_true", help="suppress per-combination benchmark progress on stderr") + b.add_argument("--merge", action="store_true", + help="replace selected audits in existing provenance-verified results") return parser @@ -932,7 +934,8 @@ def report_progress(audit, strategy, model): try: write_report(fairness_df, performance_df, args.out, make_plots=not args.no_plots, provenance=collect_provenance(fairness_df["audit"].unique(), args.n_resamples, args.n_permutations, - description="faircode benchmark")) + description="faircode benchmark"), + merge=args.merge) except ValueError as exc: print(f"error: {exc}", file=sys.stderr) return 2 diff --git a/tests/test_results_provenance.py b/tests/test_results_provenance.py index 017475bc..bda3ad5a 100644 --- a/tests/test_results_provenance.py +++ b/tests/test_results_provenance.py @@ -95,6 +95,136 @@ def test_subset_run_refuses_other_audits_without_changing_any_output(tmp_path): assert _output_bytes(out_dir) == before +def test_merge_replaces_selected_audits_preserves_other_rows_and_appends_provenance(tmp_path): + out_dir = tmp_path / "results" + _write_result_set(out_dir, ["audit_a", "audit_b"]) + fairness, performance = _result_frames(["audit_b", "audit_c"]) + fairness.loc[0, "value"] = 0.4 + performance.loc[0, "value"] = 0.8 + run = {"audits": ["audit_b", "audit_c"], "description": "selected rerun"} + + benchmark.write_report(fairness, performance, out_dir, make_plots=False, + provenance=run, merge=True) + + merged = pd.read_csv(out_dir / "results_fairness.csv").set_index("audit") + assert set(merged.index) == {"audit_a", "audit_b", "audit_c"} + assert merged.loc["audit_a", "value"] == pytest.approx(0.1) + assert merged.loc["audit_b", "value"] == pytest.approx(0.4) + for name in ("results_performance.csv", "summary.csv"): + assert set(pd.read_csv(out_dir / name)["audit"]) == {"audit_a", "audit_b", "audit_c"} + performance_merged = pd.read_csv(out_dir / "results_performance.csv").set_index("audit") + assert performance_merged.loc["audit_a", "value"] == pytest.approx(0.7) + assert performance_merged.loc["audit_b", "value"] == pytest.approx(0.8) + record = json.loads((out_dir / "PROVENANCE.json").read_text(encoding="utf-8")) + assert record["runs"] == [{"audits": ["audit_a", "audit_b"]}, run] + assert set().union(*(set(item["audits"]) for item in record["runs"])) == set(merged.index) + for name, digest in record["files"].items(): + assert hashlib.sha256((out_dir / name).read_bytes()).hexdigest() == digest + + +def test_merge_requires_provenance_and_leaves_existing_outputs_unchanged(tmp_path): + out_dir = tmp_path / "results" + _write_result_set(out_dir, ["audit_a", "audit_b"]) + before = _output_bytes(out_dir) + fairness, performance = _result_frames(["audit_b"]) + + with pytest.raises(ValueError, match="requires provenance"): + benchmark.write_report(fairness, performance, out_dir, make_plots=False, merge=True) + + assert _output_bytes(out_dir) == before + + +def test_merge_rejects_invalid_existing_output_without_changing_any_file(tmp_path): + out_dir = tmp_path / "results" + _write_result_set(out_dir, ["audit_a"]) + (out_dir / "results_performance.csv").write_text("broken", encoding="utf-8") + before = _output_bytes(out_dir) + fairness, performance = _result_frames(["audit_b"]) + + with pytest.raises(ValueError): + benchmark.write_report(fairness, performance, out_dir, make_plots=False, + provenance={"audits": ["audit_b"]}, merge=True) + + assert _output_bytes(out_dir) == before + + +def test_merge_rejects_partial_first_run_without_creating_output(tmp_path): + out_dir = tmp_path / "results" + out_dir.mkdir() + (out_dir / "results_fairness.csv").write_text("audit\naudit_a\n", encoding="utf-8") + before = _output_bytes(out_dir) + fairness, performance = _result_frames(["audit_b"]) + + with pytest.raises(ValueError, match="requires an existing, provenance-verified"): + benchmark.write_report(fairness, performance, out_dir, make_plots=False, + provenance={"audits": ["audit_b"]}, merge=True) + + assert _output_bytes(out_dir) == before + + +def test_merge_rolls_back_replaced_files_when_installation_fails(tmp_path, monkeypatch): + out_dir = tmp_path / "results" + _write_result_set(out_dir, ["audit_a"]) + before = _output_bytes(out_dir) + fairness, performance = _result_frames(["audit_b"]) + real_replace = benchmark.os.replace + failed = False + + def fail_once(source, destination): + nonlocal failed + if not failed and Path(source).name == "summary.csv" and "stage" in Path(source).parts: + failed = True + raise OSError("simulated installation failure") + return real_replace(source, destination) + + monkeypatch.setattr(benchmark.os, "replace", fail_once) + with pytest.raises(OSError, match="simulated installation failure"): + benchmark.write_report(fairness, performance, out_dir, make_plots=False, + provenance={"audits": ["audit_b"]}, merge=True) + + assert failed + assert _output_bytes(out_dir) == before + + +def test_merge_rolls_back_replaced_files_on_keyboard_interrupt(tmp_path, monkeypatch): + out_dir = tmp_path / "results" + _write_result_set(out_dir, ["audit_a"]) + before = _output_bytes(out_dir) + fairness, performance = _result_frames(["audit_b"]) + real_replace = benchmark.os.replace + interrupted = False + + def interrupt_once(source, destination): + nonlocal interrupted + if not interrupted and Path(source).name == "summary.csv" and "stage" in Path(source).parts: + interrupted = True + raise KeyboardInterrupt + return real_replace(source, destination) + + monkeypatch.setattr(benchmark.os, "replace", interrupt_once) + with pytest.raises(KeyboardInterrupt): + benchmark.write_report(fairness, performance, out_dir, make_plots=False, + provenance={"audits": ["audit_b"]}, merge=True) + + assert interrupted + assert _output_bytes(out_dir) == before + + +def test_merge_renders_figures_for_preserved_and_replaced_audits(tmp_path): + out_dir = tmp_path / "results" + _write_result_set(out_dir, ["audit_a"]) + from faircode.figures import generate_figures + + generate_figures(out_dir) + fairness, performance = _result_frames(["audit_b"]) + + benchmark.write_report(fairness, performance, out_dir, + provenance={"audits": ["audit_b"]}, merge=True) + + assert (out_dir / "figures" / "audit_a_strategies.png").is_file() + assert (out_dir / "figures" / "audit_b_strategies.png").is_file() + + def test_fresh_and_compatible_same_audit_outputs_can_be_written(tmp_path): out_dir = tmp_path / "results" _write_result_set(out_dir, ["audit_a"]) @@ -173,6 +303,21 @@ def test_benchmark_cli_reports_a_refused_subset_run_cleanly(tmp_path, monkeypatc assert _output_bytes(out_dir) == before +def test_benchmark_cli_forwards_merge_flag(tmp_path, monkeypatch): + fairness, performance = _result_frames(["audit_a"]) + received = {} + monkeypatch.setattr(benchmark, "run_benchmark", lambda **_: (fairness, performance)) + + def capture_report(*args, **kwargs): + received.update(kwargs) + + monkeypatch.setattr(benchmark, "write_report", capture_report) + from faircode.cli import main + + assert main(["benchmark", "--no-plots", "--merge", "--out", str(tmp_path / "results")]) == 0 + assert received["merge"] is True + + def test_the_committed_provenance_matches_the_committed_result_files(): record = json.loads((REPO_ROOT / "results" / "PROVENANCE.json").read_text(encoding="utf-8")) assert record["runs"], "results/PROVENANCE.json lists no run"