From 4e560b26ef38dfabc293665ad497c61b8b1e6541 Mon Sep 17 00:00:00 2001 From: Antoine Marot Date: Mon, 6 Jul 2026 14:23:39 +0000 Subject: [PATCH 1/3] perf: slim step-2 payload + serial reassessment for the 2-vCPU Space MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Analyze & Suggest on the reported first scenario (eu-pyrenees, LANNEL61PRAGN on the European grid) regressed to ~75s. Two fixes, measured with the new benchmarks/bench_analyze_suggest.py (drives /api/config -> step1 -> step2 via TestClient and decomposes the UI's "Other (network / streaming)" bucket): - Payload: each combined-action pair shipped p_or_combined / p_ex_combined — full per-line arrays (one float per grid line x ~100 pairs ~= 29 MB on the European grid) the frontend never reads (CombinedAction uses only betas / max_rho / rho_before / rho_after; session-reload rebuilds them as []). New services/analysis/combined_pairs.slim_combined_actions_for_payload() empties them at the step-2 API boundary: payload 29 269 KiB -> 267 KiB, sanitize 2.57s -> 0.01s, "Other" 3.80s -> 0.51s. - Reassessment: Dockerfile pins EXPERT_OP4GRID_REASSESSMENT_PARALLEL=0 (the library's new container-aware detection also picks serial on 2 vCPUs). - Step-2 result event now carries reassessment_parallelism so a client can confirm serial vs parallel. New benchmark + docs/performance/history/analyze-suggest-2vcpu.md. Covered by expert_backend/tests/test_combined_actions_payload_slim.py. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01XTEMAv3usezBpo2bm2L8qu Signed-off-by: Antoine Marot --- CHANGELOG.md | 28 ++ Dockerfile | 8 + benchmarks/README.md | 27 ++ benchmarks/bench_analyze_suggest.py | 387 ++++++++++++++++++ .../history/analyze-suggest-2vcpu.md | 93 +++++ .../recommenders/_service_integration.py | 12 + .../services/analysis/combined_pairs.py | 45 ++ .../test_combined_actions_payload_slim.py | 95 +++++ 8 files changed, 695 insertions(+) create mode 100644 benchmarks/bench_analyze_suggest.py create mode 100644 docs/performance/history/analyze-suggest-2vcpu.md create mode 100644 expert_backend/services/analysis/combined_pairs.py create mode 100644 expert_backend/tests/test_combined_actions_payload_slim.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e15dac3..e66f79fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,34 @@ and the project (informally) follows [Semantic Versioning](https://semver.org/). ## [Unreleased] +### Performance — Analyze & Suggest on the 2-vCPU Space (30 s → 75 s regression) + +- **Step-2 result payload no longer ships full-grid per-branch arrays.** Each + combined-action pair carried `p_or_combined` / `p_ex_combined` — one float per + line of the grid (~6–8k) × ~100 pairs ≈ **29 MB** on the European grid — that + the frontend never reads (`CombinedAction` uses only betas / max_rho / + rho_before / rho_after; session-reload rebuilds them as `[]`). New + `services/analysis/combined_pairs.slim_combined_actions_for_payload()` empties + them at the step-2 API boundary. On the reported case (`eu-pyrenees`): payload + **29 269 KiB → 267 KiB**, `sanitize_for_json` **2.57 s → 0.01 s**, and the + "Other (network / streaming)" bucket **3.80 s → 0.51 s** — plus a proportional + cut to the real browser transfer. Covered by + `tests/test_combined_actions_payload_slim.py`. +- **Reassessment forced serial on the Space** via + `EXPERT_OP4GRID_REASSESSMENT_PARALLEL=0` in the `Dockerfile`. The library's + new container-aware detection already picks serial on 2 vCPUs; the env pin + makes it explicit. (The 47 s assessment was ~10 worker threads over-subscribing + 2 vCPUs, each cloning a full network — see the `expert_op4grid_recommender` + changelog.) +- **New benchmark** `benchmarks/bench_analyze_suggest.py` drives the exact + `/api/config → step1 → step2` path via `TestClient` and prints the UI + execution-time breakdown with "Other" decomposed (discovery-overhead / + sanitize / transport); `--serial`, `--compare`, `--tier`, `--study`. Full + write-up in `docs/performance/history/analyze-suggest-2vcpu.md`. +- The step-2 `result` event now also carries `reassessment_parallelism` + (`{parallel, workers, cores_available, n_actions}`) so a client can confirm + serial vs parallel and on how many effective cores. + ### Compatibility with the `expert_op4grid_recommender` typed-pipeline refactor - **`run_analysis_step1` now tolerates the library's new single-value return.** diff --git a/Dockerfile b/Dockerfile index 1da9e378..838cdcb4 100644 --- a/Dockerfile +++ b/Dockerfile @@ -75,7 +75,15 @@ COPY --chown=user --from=frontend /build/dist ./frontend/dist COPY --chown=user frontend/src/utils/svg/pinGlyph.js ./frontend/src/utils/svg/pinGlyph.js # The backend serves this directory at "/" (same origin as the API). +# +# EXPERT_OP4GRID_REASSESSMENT_PARALLEL=0 forces the per-action reassessment to +# run SERIALLY. The Space runs on 2 vCPUs; the recommender's container-aware +# detection already picks serial there, but pinning it makes the guarantee +# explicit and independent of the host's cgroup exposure — parallel worker +# threads each clone a full pypowsybl network, so on 2 vCPUs they over-subscribe +# the CPU and are far SLOWER than serial (the 47 s → ~15 s assessment win). ENV COSTUDY4GRID_FRONTEND_DIST=/home/user/app/frontend/dist \ + EXPERT_OP4GRID_REASSESSMENT_PARALLEL=0 \ PORT=7860 EXPOSE 7860 diff --git a/benchmarks/README.md b/benchmarks/README.md index 05e3750a..883efb4d 100644 --- a/benchmarks/README.md +++ b/benchmarks/README.md @@ -36,8 +36,35 @@ export BENCH_CONTINGENCY=DISCO_NAME # only for bench_n1_diagram | `bench_n1_diagram.py` | Full `get_n1_diagram(contingency)` cold + warm, per-sub-step breakdown. Validates the 3 N-1 fast-path patches. | `docs/performance/history/n1-diagram-fast-path.md` | | `bench_nad_n_state.py` | `get_network_diagram()` cold + warm on the N-state. Captures NAD / SVG / Meta sub-timings from the `[RECO]` log lines. | `docs/performance/nad-profile-bare-env.md` | | `bench_nad_toggles.py` | Matrix of `NadParameters` toggle combinations — quantifies per-flag impact on NAD gen + SVG size, surfaces the cost of `injections_added=True`. | `docs/performance/nad-profile-bare-env.md` | +| `bench_analyze_suggest.py` | **Full "Analyze & Suggest" for a Game Mode study** — drives `/api/config` → `step1` → `step2` (streaming NDJSON) through the FastAPI `TestClient` and prints the UI's execution-time breakdown (step1 / overflow / prediction / **assessment** / enrichment / **Other**), with "Other" decomposed into discovery-overhead / result `sanitize_for_json` / transport. `--serial` forces serial reassessment; `--compare` runs parallel-vs-serial. This is the case the 30 s → 75 s regression was reported on. | `docs/performance/history/analyze-suggest-2vcpu.md` | | `run_all.py` | Drives every benchmark above sequentially. | — | +### `bench_analyze_suggest.py` + +```bash +# The reported "this case, first scenario": Pyrenees LANNEL61PRAGN on the +# medium/European grid (its network.xiidm ships as a Git-LFS zip — run +# `git lfs pull` first, or use --tier high for the uncompressed French grid). +python benchmarks/bench_analyze_suggest.py # medium tier, first study +python benchmarks/bench_analyze_suggest.py --tier high # French grid, first study +python benchmarks/bench_analyze_suggest.py --compare # parallel vs serial, same case +``` + +Two levers this benchmark validates: + +- **Per-action reassessment goes serial on a CPU-limited host.** The tail line + reports `reassessment: serial|parallel — N worker(s) / M effective core(s)`. + On a 2-vCPU host the recommender's container-aware detection picks serial; + even on a 4-core dev box `--compare` shows parallel is no faster than serial + (each worker clones a full network), so over-subscribing 2 vCPUs with ~10 + workers was the 47 s assessment in the regression. +- **The step-2 result payload no longer ships full-grid per-branch arrays.** + Each combined-action pair used to carry `p_or_combined` / `p_ex_combined` + (one float per line × ~100 pairs ≈ **29 MB** on the European grid); the + frontend reads neither, so they are emptied at the API boundary. Watch + `payload=… KiB` and the `result sanitize_for_json` sub-line drop + (29 269 KiB / 2.57 s → 267 KiB / 0.01 s). + ## Reference measurements On a developer box with pypowsybl 1.14.0 + Python 3.12 + the full diff --git a/benchmarks/bench_analyze_suggest.py b/benchmarks/bench_analyze_suggest.py new file mode 100644 index 00000000..d141d0db --- /dev/null +++ b/benchmarks/bench_analyze_suggest.py @@ -0,0 +1,387 @@ +#!/usr/bin/env python3 +# Copyright (c) 2025-2026, RTE (https://www.rte-france.com) +# This Source Code Form is subject to the terms of the Mozilla Public License, version 2.0. +# If a copy of the Mozilla Public License, version 2.0 was not distributed with this file, +# you can obtain one at http://mozilla.org/MPL/2.0/. +# SPDX-License-Identifier: MPL-2.0 + +"""Full "Analyze & Suggest" benchmark for a Game Mode study. + +Drives the exact code path the Game Mode UI runs for one study — +``POST /api/config`` → ``/api/run-analysis-step1`` → ``/api/run-analysis-step2`` +(streaming NDJSON) — through the FastAPI ``TestClient`` (in-process, no port), +and reports the **same per-stage breakdown the UI shows** in its +"Execution time breakdown" tooltip: + + Step 1 (contingency simulation) ... step1_time + Overflow analysis ................. overflow_graph_time + Action prediction ................. action_prediction_time + Action assessment ................. assessment_time (per-action reassessment) + Enrichment / post-process ......... enrichment_time + Other (network / streaming) ....... wall − sum(the five above) + Total (wall-clock, click → display) wall + +Unlike the UI, it further **decomposes "Other"** into the server-side pieces +that fall outside the reported stages (discovery overhead = expert-rule filter ++ input building; result-payload ``sanitize_for_json``; NDJSON size), so a +regression there can be attributed instead of hiding in a single bucket. + +This is the case the perf work targets: the Game "first scenario" run that +regressed from ~30 s to ~75 s on the 2-vCPU HuggingFace Space. Use it to: + +- confirm the per-action reassessment goes **serial** on a CPU-limited host + (``--serial`` forces it; the tail line reports workers / effective cores); +- compare parallel vs serial assessment (``--compare``); +- watch the "Other" residual after a serialization / filtering change. + +Usage (from the repo root, backend venv active):: + + python benchmarks/bench_analyze_suggest.py # medium tier, first study + python benchmarks/bench_analyze_suggest.py --tier high # French grid, first study + python benchmarks/bench_analyze_suggest.py --study s3 # a specific study id + python benchmarks/bench_analyze_suggest.py --serial # force serial reassessment + python benchmarks/bench_analyze_suggest.py --compare # parallel vs serial, same case + +Path overrides (when the study's grid is not on disk — e.g. the medium/European +grid ships as a Git-LFS ``network.xiidm.zip`` that must be pulled first):: + + BENCH_NETWORK_PATH=data/pypsa_eur_fr225_400/network.xiidm \\ + BENCH_ACTION_FILE=data/pypsa_eur_fr225_400/actions.json \\ + BENCH_LAYOUT=data/pypsa_eur_fr225_400/grid_layout.json \\ + BENCH_CONTINGENCY=way_109818602-225 \\ + python benchmarks/bench_analyze_suggest.py +""" +from __future__ import annotations + +import argparse +import json +import os +import sys +import time +from pathlib import Path +from typing import Any + +_REPO_ROOT = Path(__file__).resolve().parent.parent +if str(_REPO_ROOT) not in sys.path: + sys.path.insert(0, str(_REPO_ROOT)) + +# --------------------------------------------------------------------------- +# Study presets — a Python mirror of the two tiers in +# `frontend/src/game/presets.ts` (kept in sync). The default is the medium +# tier's FIRST study, matching the Game Config screen's default and the case +# the perf regression was reported on. The medium/European grid ships as a +# Git-LFS zip; if it is not pulled, pass --tier high (the French grid ships +# uncompressed) or set the BENCH_* path overrides. +# --------------------------------------------------------------------------- +_EUR = { + "network": "data/pypsa_eur_eur220_225_380_400/network.xiidm", + "actions": "data/pypsa_eur_eur220_225_380_400/actions.json", + "layout": "data/pypsa_eur_eur220_225_380_400/grid_layout.json", +} +_FR = { + "network": "data/pypsa_eur_fr225_400/network.xiidm", + "actions": "data/pypsa_eur_fr225_400/actions.json", + "layout": "data/pypsa_eur_fr225_400/grid_layout.json", +} + +TIERS: dict[str, dict[str, Any]] = { + "medium": { + "paths": _EUR, + "studies": [ + {"id": "eu-pyrenees", "label": "Pyrenees 225 kV — LANNEL61PRAGN", + "contingency": "relation_8423570-225"}, + {"id": "eu-italy", "label": "Campania 380 kV — Santa Sofia", + "contingency": "relation_13164355-380"}, + {"id": "eu-spain", "label": "Hinojosa 400 kV double line", + "contingency": "way_170479605-400"}, + ], + }, + "high": { + "paths": _FR, + "studies": [ + {"id": "s1", "label": "Toulouse 225 kV — Saint-Orens - Verfeil", + "contingency": "way_109818602-225"}, + {"id": "s2", "label": "Biancon 225 kV — way/121500507", + "contingency": "way_121500507-225"}, + {"id": "s3", "label": "Valence 225 kV — B.MONL61VALE8", + "contingency": "relation_6028666_c-225"}, + {"id": "s4", "label": "Breuil 225 kV — BREUIL63CHAST", + "contingency": "relation_8307566_d-225"}, + {"id": "s5", "label": "way/1463717755 225 kV", + "contingency": "way_1463717755-225"}, + {"id": "s6", "label": "Échalas 225 kV — Échalas - Le Soleil", + "contingency": "way_130969307-225"}, + {"id": "s7", "label": "Génissiat 400 kV — Cornier - Génissiat", + "contingency": "merged_way_100497456-400_1"}, + {"id": "s8", "label": "Villejust 225 kV — Liers - Villejust", + "contingency": "way_204035714-225"}, + ], + }, +} + +# Recommender config mirrors config.default.json (the bundled first-run +# settings the HuggingFace Space boots with). +CONFIG_MINIMA = { + "min_line_reconnections": 2.0, + "min_close_coupling": 3.0, + "min_open_coupling": 2.0, + "min_line_disconnections": 3.0, + "min_pst": 1.0, + "min_load_shedding": 2.0, + "min_renewable_curtailment_actions": 2, + "min_redispatch": 2, + "n_prioritized_actions": 15, + "monitoring_factor": 0.95, + "pre_existing_overload_threshold": 0.02, + "ignore_reconnections": False, + "pypowsybl_fast_mode": True, +} + + +def _resolve_case(args) -> tuple[dict, dict]: + """Return ``(paths, study)`` after applying tier/study selection + env + overrides.""" + tier = TIERS[args.tier] + paths = dict(tier["paths"]) + studies = tier["studies"] + if args.study: + match = [s for s in studies if s["id"] == args.study] + if not match: + raise SystemExit( + f"study '{args.study}' not in tier '{args.tier}'. " + f"Available: {', '.join(s['id'] for s in studies)}" + ) + study = dict(match[0]) + else: + study = dict(studies[0]) + + # Env overrides let the benchmark run when a study's grid is not on disk. + paths["network"] = os.environ.get("BENCH_NETWORK_PATH", paths["network"]) + paths["actions"] = os.environ.get("BENCH_ACTION_FILE", paths["actions"]) + paths["layout"] = os.environ.get("BENCH_LAYOUT", paths["layout"]) + study["contingency"] = os.environ.get("BENCH_CONTINGENCY", study["contingency"]) + return paths, study + + +class _Instrument: + """Monkeypatches that attribute the server-side share of "Other". + + - ``sanitize_for_json`` (result-payload coercion at the NDJSON yield) + - ``run_analysis_step2_discovery`` wall time (so the discovery overhead — + expert-rule filter + recommender-input build, which is NOT part of the + reported prediction / assessment split — can be isolated). + """ + + def __init__(self): + self.sanitize_s = 0.0 + self.discovery_wall_s = 0.0 + self._patches = [] + + def __enter__(self): + from expert_backend.recommenders import _service_integration as si + from expert_backend.services import analysis_mixin as am + + orig_sanitize = si.sanitize_for_json + + def timed_sanitize(obj): + t0 = time.perf_counter() + try: + return orig_sanitize(obj) + finally: + self.sanitize_s += time.perf_counter() - t0 + + orig_disc = am.run_analysis_step2_discovery + + def timed_discovery(*a, **kw): + t0 = time.perf_counter() + try: + return orig_disc(*a, **kw) + finally: + self.discovery_wall_s += time.perf_counter() - t0 + + si.sanitize_for_json = timed_sanitize + am.run_analysis_step2_discovery = timed_discovery + self._patches = [ + (si, "sanitize_for_json", orig_sanitize), + (am, "run_analysis_step2_discovery", orig_disc), + ] + return self + + def __exit__(self, *exc): + for mod, name, orig in self._patches: + setattr(mod, name, orig) + return False + + +def run_once(client, paths, study, reset_between=True) -> dict: + """Drive config→step1→step2 for one study; return a metrics dict.""" + cfg = { + "network_path": paths["network"], + "action_file_path": paths["actions"], + "layout_path": paths["layout"], + "model": "expert", + "compute_overflow_graph": True, + **CONFIG_MINIMA, + } + t_cfg = time.perf_counter() + r = client.post("/api/config", json=cfg) + r.raise_for_status() + t_cfg = time.perf_counter() - t_cfg + + contingency = study["contingency"] + t_s1 = time.perf_counter() + r = client.post("/api/run-analysis-step1", + json={"disconnected_elements": [contingency]}) + r.raise_for_status() + t_s1 = time.perf_counter() - t_s1 + step1 = r.json() + overloads = step1.get("lines_overloaded", []) + if not (step1.get("can_proceed") and overloads): + return {"error": f"no actionable overload (can_proceed={step1.get('can_proceed')}, " + f"{len(overloads)} overload(s))"} + + with _Instrument() as inst: + t_s2 = time.perf_counter() + r = client.post("/api/run-analysis-step2", + json={"selected_overloads": overloads, "all_overloads": overloads}) + r.raise_for_status() + payload_text = r.text + t_s2 = time.perf_counter() - t_s2 + + result = {} + for line in payload_text.splitlines(): + line = line.strip() + if not line: + continue + ev = json.loads(line) + if ev.get("type") == "result": + result = ev + elif ev.get("type") == "error": + return {"error": ev.get("message")} + + # Per-stage timings reported by the backend (the UI tooltip buckets). + step1_t = result.get("step1_time") or 0.0 + overflow_t = result.get("overflow_graph_time") or 0.0 + prediction_t = result.get("action_prediction_time") or 0.0 + assessment_t = result.get("assessment_time") or 0.0 + enrichment_t = result.get("enrichment_time") or 0.0 + backend_sum = step1_t + overflow_t + prediction_t + assessment_t + enrichment_t + + # Wall-clock "click → display" proxy: step1 + step2 request wall time. + wall = t_s1 + t_s2 + other = max(0.0, wall - backend_sum) + discovery_overhead = max(0.0, inst.discovery_wall_s - prediction_t - assessment_t) + + reassess = result.get("reassessment_parallelism") + if reassess is None: + from expert_backend.services.recommender_service import recommender_service + reassess = (getattr(recommender_service, "_last_result", None) or {}).get( + "reassessment_parallelism") + + return { + "n_actions": len(result.get("actions", {})), + "n_overloads": len(overloads), + "payload_bytes": len(payload_text), + "t_config": t_cfg, + "t_step1_http": t_s1, + "t_step2_http": t_s2, + "wall": wall, + "step1": step1_t, + "overflow": overflow_t, + "prediction": prediction_t, + "assessment": assessment_t, + "enrichment": enrichment_t, + "backend_sum": backend_sum, + "other": other, + "other_discovery_overhead": discovery_overhead, + "other_sanitize": inst.sanitize_s, + "other_residual": max(0.0, other - discovery_overhead - inst.sanitize_s), + "reassessment": reassess, + } + + +def _fmt(s: float) -> str: + return f"{s:6.2f}s" + + +def _print_breakdown(m: dict, title: str) -> None: + if "error" in m: + print(f"\n{title}: SKIPPED — {m['error']}") + return + total = m["wall"] + + def pct(x): + return f"{100 * x / total:4.0f}%" if total else " -" + print(f"\n=== {title} ===") + print(f" actions={m['n_actions']} overloads={m['n_overloads']} " + f"payload={m['payload_bytes'] / 1024:.0f} KiB config-load={_fmt(m['t_config'])}") + print(f" {'Step 1 (contingency simulation)':<34} {_fmt(m['step1'])} {pct(m['step1'])}") + print(f" {'Overflow analysis':<34} {_fmt(m['overflow'])} {pct(m['overflow'])}") + print(f" {'Action prediction':<34} {_fmt(m['prediction'])} {pct(m['prediction'])}") + print(f" {'Action assessment (reassessment)':<34} {_fmt(m['assessment'])} {pct(m['assessment'])}") + print(f" {'Enrichment / post-process':<34} {_fmt(m['enrichment'])} {pct(m['enrichment'])}") + print(f" {'Other (network / streaming)':<34} {_fmt(m['other'])} {pct(m['other'])}") + print(f" ├─ {'discovery overhead (filter+inputs)':<29} {_fmt(m['other_discovery_overhead'])}") + print(f" ├─ {'result sanitize_for_json':<29} {_fmt(m['other_sanitize'])}") + print(f" └─ {'transport / frontend residual':<29} {_fmt(m['other_residual'])}") + print(f" {'─' * 46}") + print(f" {'Total (wall-clock, click → display)':<34} {_fmt(total)}") + if m.get("reassessment"): + ra = m["reassessment"] + mode = "parallel" if ra.get("parallel") else "serial" + print(f" reassessment: {mode} — {ra.get('workers')} worker(s) / " + f"{ra.get('cores_available')} effective core(s), " + f"{ra.get('n_actions')} action(s)") + + +def main() -> None: + ap = argparse.ArgumentParser(description=__doc__, + formatter_class=argparse.RawDescriptionHelpFormatter) + ap.add_argument("--tier", choices=list(TIERS), default="medium", + help="difficulty tier (default: medium = European grid)") + ap.add_argument("--study", default=None, + help="study id within the tier (default: first study)") + ap.add_argument("--reps", type=int, default=1, + help="repetitions (median reported); a fresh config load per rep") + ap.add_argument("--serial", action="store_true", + help="force serial per-action reassessment (EXPERT_OP4GRID_REASSESSMENT_PARALLEL=0)") + ap.add_argument("--compare", action="store_true", + help="run the same case twice — parallel then serial — and print both") + args = ap.parse_args() + + os.chdir(_REPO_ROOT) + from fastapi.testclient import TestClient + + from expert_backend.main import app + from expert_op4grid_recommender import config as eo_config + + paths, study = _resolve_case(args) + net_ok = Path(paths["network"]).is_file() and Path(paths["network"]).stat().st_size > 1024 + print(f"Case: tier={args.tier} study={study['id']} ({study['label']})") + print(f" network={paths['network']} contingency={study['contingency']}") + if not net_ok: + print(f"\nWARNING: {paths['network']} is missing or a Git-LFS pointer.\n" + f" Pull LFS (git lfs pull) or use --tier high / the BENCH_* path overrides.") + + def _run(force_serial: bool, label: str) -> dict: + eo_config.REASSESSMENT_PARALLEL = False if force_serial else None + samples = [] + with TestClient(app) as client: + for _ in range(max(1, args.reps)): + samples.append(run_once(client, paths, study)) + ok = [s for s in samples if "error" not in s] + if not ok: + return samples[0] + ok.sort(key=lambda s: s["wall"]) + return ok[len(ok) // 2] # median by wall-clock + + if args.compare: + _print_breakdown(_run(False, "parallel"), "PARALLEL (auto)") + _print_breakdown(_run(True, "serial"), "SERIAL (forced)") + else: + _print_breakdown(_run(args.serial, "run"), + f"{'SERIAL (forced)' if args.serial else 'AUTO'}") + + +if __name__ == "__main__": + main() diff --git a/docs/performance/history/analyze-suggest-2vcpu.md b/docs/performance/history/analyze-suggest-2vcpu.md new file mode 100644 index 00000000..d7db04cb --- /dev/null +++ b/docs/performance/history/analyze-suggest-2vcpu.md @@ -0,0 +1,93 @@ +# Analyze & Suggest on a 2-vCPU Space — reassessment serial + payload slim + +## Context + +The Game Mode "first scenario" (Pyrenees `LANNEL61PRAGN`, +`relation_8423570-225`, on the medium/European +`pypsa_eur_eur220_225_380_400` grid) regressed from ~30 s to ~75 s on the +2-vCPU HuggingFace Space. The in-app execution-time breakdown showed: + +| Stage | Reported | +|---|---| +| Step 1 (contingency simulation) | 1.84 s | +| Overflow analysis | 8.27 s | +| Action prediction | 2.72 s | +| **Action assessment (reassessment)** | **47.0 s** | +| Enrichment / post-process | 2.20 s | +| **Other (network / streaming)** | **12.8 s** | +| **Total (click → display)** | **74.8 s** | + +Two culprits: a 47 s reassessment and a 12.8 s "Other". Both were measured +and fixed with `benchmarks/bench_analyze_suggest.py`, which drives the exact +`/api/config → step1 → step2` path via `TestClient` and decomposes "Other". + +## 1. Reassessment: parallel → serial on a CPU-limited host + +`utils/reassessment.py` (in `expert_op4grid_recommender`) parallelised the +per-action re-simulation across `min(10, os.cpu_count(), n_actions)` worker +threads, each cloning a **full private pypowsybl network**. The Space log read: + +``` +[Reassessment] 15 action(s) re-simulated on 10/16 core(s) (parallel). +[Timer] Reassessment took 48.5592s +``` + +`os.cpu_count()` reports the **host** core count (16), not the container's +2-vCPU allocation. So 10 worker threads over-subscribed 2 vCPUs *and* paid the +per-worker network-clone tax → far slower than serial. + +**Fix** (`expert_op4grid_recommender`): + +- `_effective_cpu_count()` — container-aware CPU detection: the min of + `os.cpu_count()`, the scheduler affinity mask, and the **cgroup CPU quota** + (`cpu.max` / `cpu.cfs_quota_us`). Returns 2 on the Space, not 16. +- `_reassessment_worker_count()` engages parallel only when it pays off, + gated by two config knobs (env-overridable via `EXPERT_OP4GRID_*`): + `REASSESSMENT_PARALLEL` (None=auto / True / False) and + `REASSESSMENT_MIN_PARALLEL_CORES` (default 4). On ≤ 3 effective cores → serial. +- The Space `Dockerfile` also pins `EXPERT_OP4GRID_REASSESSMENT_PARALLEL=0` as a + belt-and-suspenders guarantee independent of cgroup exposure. + +`--compare` on a 4-core dev box shows parallel is **no faster than serial** +even there (assessment 14.27 s parallel vs 13.93 s serial) — the clone tax +cancels the concurrency — so on 2 vCPUs the ~10-worker pool was pure loss. +Expected Space assessment: ~47 s → ~15 s. + +## 2. "Other (network / streaming)": a 29 MB payload + +`bench_analyze_suggest.py` decomposes "Other" into discovery-overhead / +result-`sanitize_for_json` / transport. On the European case the step-2 result +payload was **29 MB** and `sanitize_for_json` alone took 2.57 s. The cause: +every combined-action pair carries `p_or_combined` / `p_ex_combined` — the +superposed per-branch active-power vectors, **one float per line of the grid** +(~6–8k) × ~100 pairs. The frontend reads **none** of them (`CombinedAction` +uses only betas / max_rho / rho_before / rho_after; session-reload rebuilds +them as `[]`). + +**Fix** (`Co-Study4Grid`): `services/analysis/combined_pairs.py` +`slim_combined_actions_for_payload()` empties those two keys at the step-2 API +boundary (emptied, not deleted, so the shape matches a reloaded session). + +## Result (European `eu-pyrenees`, serial, 4-core dev box) + +| Metric | Before | After | +|---|---|---| +| Result payload | 29 269 KiB | **267 KiB** (−99 %) | +| `sanitize_for_json` | 2.57 s | **0.01 s** | +| Other (network / streaming) | 3.80 s | **0.51 s** | + +(The residual 0.49 s of "Other" is now the expert rule-filter + recommender-input +build — real compute that runs inside the discovery call but outside the +reported prediction/assessment split.) + +The payload shrink also slashes the real-browser transfer of the 29 MB body — +the dominant part of the 12.8 s "Other" the operator saw on the Space, which +`TestClient` (in-process) under-measures. + +## Reproduce + +```bash +git lfs pull # the European network.xiidm ships as a Git-LFS zip +python benchmarks/bench_analyze_suggest.py --compare # medium tier, first study +python benchmarks/bench_analyze_suggest.py --tier high --serial # French grid +``` diff --git a/expert_backend/recommenders/_service_integration.py b/expert_backend/recommenders/_service_integration.py index e6ab223c..6bcdb9e0 100644 --- a/expert_backend/recommenders/_service_integration.py +++ b/expert_backend/recommenders/_service_integration.py @@ -30,6 +30,9 @@ from typing import Any from expert_backend.recommenders.registry import build_recommender +from expert_backend.services.analysis.combined_pairs import ( + slim_combined_actions_for_payload, +) from expert_backend.services.model_selection_mixin import ModelSelectionMixin from expert_backend.services.recommender_service import ( RecommenderService, @@ -278,6 +281,10 @@ def _run_analysis_step2_with_model( # `target_max_rho` decoration; for random models the call is a # no-op (combined_actions is empty), so we keep it unconditional. self._augment_combined_actions_with_target_max_rho(results, context) + # Drop the per-branch full-grid arrays the frontend never reads BEFORE + # they hit sanitize_for_json / the wire — the single biggest lever on + # the "Other (network / streaming)" time on large grids. + slim_combined_actions_for_payload(results.get("combined_actions")) action_scores = self._compute_mw_start_for_scores( results.get("action_scores", {}) ) @@ -311,6 +318,11 @@ def _run_analysis_step2_with_model( "overflow_graph_time": overflow_graph_time, "action_prediction_time": action_prediction_time, "assessment_time": assessment_time, + # How the per-action reassessment was parallelised — lets the client + # confirm serial vs parallel and on how many effective cores + # ({"parallel": bool, "workers": int, "cores_available": int, + # "n_actions": int}). None on older recommender releases. + "reassessment_parallelism": results.get("reassessment_parallelism"), }) except Exception as e: logger.exception("Backend Error in Analysis Resolution") diff --git a/expert_backend/services/analysis/combined_pairs.py b/expert_backend/services/analysis/combined_pairs.py new file mode 100644 index 00000000..be6e1ce8 --- /dev/null +++ b/expert_backend/services/analysis/combined_pairs.py @@ -0,0 +1,45 @@ +# Copyright (c) 2025-2026, RTE (https://www.rte-france.com) +# This Source Code Form is subject to the terms of the Mozilla Public License, version 2.0. +# If a copy of the Mozilla Public License, version 2.0 was not distributed with this file, +# you can obtain one at http://mozilla.org/MPL/2.0/. +# SPDX-License-Identifier: MPL-2.0 +# This file is part of Co-Study4Grid a Power Grid Study tool Assistant Interface to help solve contigencies for a grid state under study. + +"""Combined-action (superposition pair) payload shaping. + +Pure helper kept out of ``analysis_mixin`` / ``_service_integration`` so it is +independently testable (no pypowsybl / recommender import chain). +""" +from __future__ import annotations + +from typing import Any, Dict, Optional + +# Per-branch full-grid vectors the recommender embeds in every combined pair +# (``p_or_combined`` / ``p_ex_combined`` = one float PER LINE of the grid, +# ~6–8k on the PyPSA-EUR grids). The Co-Study4Grid frontend reads NONE of them +# — ``CombinedAction`` uses only betas / max_rho / rho_before / rho_after, and +# session-reload rebuilds these as ``[]`` — yet at ~100 pairs they are the bulk +# of the step-2 result payload (≈ 29 MB on the European grid), inflating both +# ``sanitize_for_json`` and the browser transfer (the "Other (network / +# streaming)" bucket in the execution-time breakdown). +HEAVY_PAIR_ARRAY_KEYS = ("p_or_combined", "p_ex_combined") + + +def slim_combined_actions_for_payload( + combined_actions: Optional[Dict[str, Any]], +) -> Optional[Dict[str, Any]]: + """Empty the full-grid per-branch arrays from each combined pair in place. + + Empties (does NOT delete) the keys so the payload shape is unchanged and any + consumer probing for the key still finds it — matching exactly what a + saved-then-reloaded session already looks like. Returns the same mapping for + call-site convenience; safe on ``None`` / non-dict entries. + """ + if not combined_actions: + return combined_actions + for pair in combined_actions.values(): + if isinstance(pair, dict): + for key in HEAVY_PAIR_ARRAY_KEYS: + if pair.get(key): + pair[key] = [] + return combined_actions diff --git a/expert_backend/tests/test_combined_actions_payload_slim.py b/expert_backend/tests/test_combined_actions_payload_slim.py new file mode 100644 index 00000000..efa8b4a9 --- /dev/null +++ b/expert_backend/tests/test_combined_actions_payload_slim.py @@ -0,0 +1,95 @@ +# Copyright (c) 2025-2026, RTE (https://www.rte-france.com) +# This Source Code Form is subject to the terms of the Mozilla Public License, version 2.0. +# If a copy of the Mozilla Public License, version 2.0 was not distributed with this file, +# you can obtain one at http://mozilla.org/MPL/2.0/. +# SPDX-License-Identifier: MPL-2.0 +"""Payload slimming: the step-2 result must not ship full-grid per-branch +arrays (``p_or_combined`` / ``p_ex_combined``) the frontend never reads. + +These arrays are one float per line of the grid (~6–8k on the PyPSA-EUR grids) +× ~100 combined pairs = tens of MB of JSON, the dominant chunk of the +"Other (network / streaming)" time. The frontend's ``CombinedAction`` reads +none of them (session-reload rebuilds them as ``[]``), so the backend empties +them at the API boundary. +""" +from expert_backend.services.analysis.combined_pairs import ( + slim_combined_actions_for_payload as _slim_combined_actions_for_payload, +) + + +def _pair(p_or, p_ex, **extra): + d = { + "betas": [0.5, 0.5], + "max_rho": 1.1, + "max_rho_line": "L1", + "rho_before": [1.2], + "rho_after": [1.05], + "p_or_combined": p_or, + "p_ex_combined": p_ex, + } + d.update(extra) + return d + + +def test_heavy_arrays_emptied(): + combined = {"a+b": _pair(list(range(8000)), list(range(8000)))} + _slim_combined_actions_for_payload(combined) + assert combined["a+b"]["p_or_combined"] == [] + assert combined["a+b"]["p_ex_combined"] == [] + + +def test_key_is_kept_not_deleted(): + """Emptied, not removed — shape stays identical to a reloaded session.""" + combined = {"a+b": _pair([1.0, 2.0], [3.0, 4.0])} + _slim_combined_actions_for_payload(combined) + assert "p_or_combined" in combined["a+b"] + assert "p_ex_combined" in combined["a+b"] + + +def test_light_fields_untouched(): + combined = {"a+b": _pair([1.0] * 8000, [2.0] * 8000, + target_max_rho=0.9, is_rho_reduction=True)} + _slim_combined_actions_for_payload(combined) + p = combined["a+b"] + assert p["betas"] == [0.5, 0.5] + assert p["max_rho"] == 1.1 + assert p["rho_before"] == [1.2] + assert p["rho_after"] == [1.05] + assert p["target_max_rho"] == 0.9 + assert p["is_rho_reduction"] is True + + +def test_multiple_pairs(): + combined = { + "a+b": _pair([1] * 100, [2] * 100), + "c+d": _pair([3] * 100, [4] * 100), + } + _slim_combined_actions_for_payload(combined) + assert all(p["p_or_combined"] == [] and p["p_ex_combined"] == [] + for p in combined.values()) + + +def test_empty_or_none_is_noop(): + assert _slim_combined_actions_for_payload({}) == {} + assert _slim_combined_actions_for_payload(None) is None + + +def test_already_empty_pair_stays_empty(): + combined = {"a+b": _pair([], [])} + _slim_combined_actions_for_payload(combined) + assert combined["a+b"]["p_or_combined"] == [] + + +def test_non_dict_pair_skipped(): + """Defensive: never raise on an unexpected value shape.""" + combined = {"a+b": None, "c+d": _pair([1] * 10, [2] * 10)} + _slim_combined_actions_for_payload(combined) + assert combined["a+b"] is None + assert combined["c+d"]["p_or_combined"] == [] + + +def test_missing_error_pair_untouched(): + """An error pair (no arrays) passes through unharmed.""" + combined = {"a+b": {"betas": [0.5, 0.5], "error": "unreliable"}} + _slim_combined_actions_for_payload(combined) + assert combined["a+b"] == {"betas": [0.5, 0.5], "error": "unreliable"} From c5e64e3b0f6d351969e41bd8f390971267f9ef4f Mon Sep 17 00:00:00 2001 From: Antoine Marot Date: Mon, 6 Jul 2026 14:23:39 +0000 Subject: [PATCH 2/3] docs: document ainetus-upstream PR workflow + DCO sign-off convention Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01XTEMAv3usezBpo2bm2L8qu Signed-off-by: Antoine Marot --- CLAUDE.md | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index 35655876..84e40345 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -429,6 +429,40 @@ NAD/SLD payloads: --- +## Contributing & Pull Requests + +- **Upstream is `ainetus`; `marota` is the working fork.** Development branches + are pushed to `marota/Co-Study4Grid`, but **pull requests are opened directly + against the upstream `ainetus/Co-Study4Grid`** (base = its default branch, + head = `marota:`) — *not* against `marota`. The sibling library + `Expert_op4grid_recommender` follows the same rule against + `ainetus/Expert_op4grid_recommender`. +- **Load `ainetus` as an initial source.** A cross-fork PR into `ainetus` can + only be created from a session/tool context that has the `ainetus` repo in + scope, so a new working session should be started with + **`ainetus/Co-Study4Grid` and `ainetus/Expert_op4grid_recommender` as the + initial sources** (they should always be auto-loaded). A session rooted only + at `marota` cannot target `ainetus` (cross-tier adds are blocked) and the PR + step fails with an access-denied error. +- **DCO sign-off is required on every commit.** The `ainetus` repos enforce the + [Developer Certificate of Origin](https://developercertificate.org/): every + commit must carry a `Signed-off-by: ` trailer, and + because the DCO check matches the sign-off against the commit **author**, the + commit must also be *authored* under that same identity (author email = + `amarot91@gmail.com`): + + ```bash + git config user.name "" + git config user.email "amarot91@gmail.com" + git commit -s -m "..." # -s appends the Signed-off-by trailer + ``` + + To sign off commits already made under a different identity, re-author and add + the trailer (`git rebase --exec 'git commit --amend --no-edit --reset-author \ + -s' `), then force-with-lease push. + +--- + ## Standalone Interface Parity Audit The detailed audit — feature inventory, mirror-status table, Layer From bde014ac2545ef4244f6dcc67f56e0cbc0fd7a00 Mon Sep 17 00:00:00 2001 From: Antoine Marot Date: Mon, 6 Jul 2026 15:20:48 +0000 Subject: [PATCH 3/3] docs: add marota<->ainetus fork-sync instruction to the Contributing section Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01XTEMAv3usezBpo2bm2L8qu Signed-off-by: Antoine Marot --- CLAUDE.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index 84e40345..77039fba 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -444,6 +444,16 @@ NAD/SLD payloads: initial sources** (they should always be auto-loaded). A session rooted only at `marota` cannot target `ainetus` (cross-tier adds are blocked) and the PR step fails with an access-denied error. +- **Sync `marota` with `ainetus` before starting new work.** PRs merge into + `ainetus/main`, but development happens on `marota`, so `marota/main` drifts + behind `ainetus/main` after every merged PR (this applies to **both** repos — + Co-Study4Grid and Expert_op4grid_recommender). **At the start of a dev session, + bring `marota/main` up to date with `ainetus/main`** — GitHub "Sync fork", or + locally `git fetch ainetus main && git merge --ff-only ainetus/main` then push + `marota/main` — and branch from there. Skipping this makes a new branch collide + with the already-merged revisions when it is PR'd into `ainetus`. If the sync + was missed and the PR already shows conflicts, merge `ainetus/main` into the + branch (or rebase onto it) and resolve, then force-with-lease push. - **DCO sign-off is required on every commit.** The `ainetus` repos enforce the [Developer Certificate of Origin](https://developercertificate.org/): every commit must carry a `Signed-off-by: ` trailer, and