diff --git a/.claude/hooks/init-sync-check.sh b/.claude/hooks/init-sync-check.sh new file mode 100755 index 00000000..4323a3a1 --- /dev/null +++ b/.claude/hooks/init-sync-check.sh @@ -0,0 +1,89 @@ +#!/usr/bin/env bash +# +# SessionStart init check — git sync verification for marota-fork sessions. +# +# Runs automatically at the start of every Claude Code session. It performs a +# REAL `git fetch` of the repository's default branch and reports whether the +# current working branch is in sync with it (ahead / behind counts), so a new +# session never assumes it is up to date based on a possibly-stale local +# `origin/` ref. +# +# Design contract: +# * READ-ONLY — never mutates the working tree, index, or any branch +# (only `git fetch`, which updates remote-tracking refs). +# * NON-FATAL — any failure (no network, no git, detached HEAD, …) exits 0 +# so it can never block or break session startup. +# * SCOPED — only emits the detailed check for `marota/*` origins, per +# the "starting a new session from a marota fork" request; +# a no-op for every other remote. +# * SYNCHRONOUS — fast (one time-boxed fetch); stdout is surfaced to the +# session as startup context. +# +set -uo pipefail + +# Drain hook stdin (SessionStart passes a JSON payload we don't need) so the +# pipe closes cleanly. +cat >/dev/null 2>&1 || true + +repo_root="${CLAUDE_PROJECT_DIR:-$(git rev-parse --show-toplevel 2>/dev/null || true)}" +[ -z "${repo_root}" ] && exit 0 +cd "${repo_root}" 2>/dev/null || exit 0 + +# Only act inside a git work tree. +git rev-parse --is-inside-work-tree >/dev/null 2>&1 || exit 0 + +origin_url="$(git remote get-url origin 2>/dev/null || true)" + +# Guard: only run for marota forks. Everything else is a silent no-op. +case "${origin_url}" in + *marota/*) : ;; + *) exit 0 ;; +esac + +# Resolve the default branch: origin/HEAD -> `git remote show` -> main. +default_branch="$(git symbolic-ref --quiet --short refs/remotes/origin/HEAD 2>/dev/null | sed 's#^origin/##')" +if [ -z "${default_branch}" ]; then + default_branch="$(git remote show origin 2>/dev/null | sed -n 's/.*HEAD branch: //p' | head -1)" +fi +[ -z "${default_branch}" ] && default_branch="main" + +current_branch="$(git rev-parse --abbrev-ref HEAD 2>/dev/null || echo '(detached)')" + +# Do a REAL fetch — never trust the local origin ref. Time-boxed + non-fatal. +fetch_status="ok" +if command -v timeout >/dev/null 2>&1; then + timeout 60 git fetch --quiet origin "${default_branch}" 2>/dev/null || fetch_status="failed" +else + git fetch --quiet origin "${default_branch}" 2>/dev/null || fetch_status="failed" +fi + +counts="$(git rev-list --left-right --count "origin/${default_branch}...HEAD" 2>/dev/null || true)" +behind="$(printf '%s' "${counts}" | awk '{print $1}')" +ahead="$(printf '%s' "${counts}" | awk '{print $2}')" + +echo "── Session init: git sync check (marota fork) ─────────────────────────" +echo "repo: $(basename "${repo_root}")" +echo "current branch: ${current_branch}" +echo "default branch: origin/${default_branch}" +echo "origin fetch: ${fetch_status}" +if [ -n "${counts}" ]; then + echo "vs origin/${default_branch}: ${behind:-?} behind, ${ahead:-?} ahead" + if [ "${behind:-0}" -gt 0 ] 2>/dev/null; then + echo "ACTION: branch is BEHIND origin/${default_branch} by ${behind} commit(s)." + echo " Rebase onto the latest default before pushing follow-up work:" + echo " git fetch origin ${default_branch} && git rebase origin/${default_branch}" + elif [ "${ahead:-0}" -eq 0 ] 2>/dev/null; then + echo "STATUS: in sync — branch tip equals origin/${default_branch}" + echo " (fresh branch, no unmerged work). If a prior PR for this branch" + echo " was already merged, treat new work as a fresh change on this base." + else + echo "STATUS: ${ahead} local commit(s) ahead of origin/${default_branch}, 0 behind — up to date." + fi +else + echo "NOTE: could not compute ahead/behind (origin/${default_branch} missing or fetch failed)." +fi +echo "PRINCIPLE: always run a real 'git fetch' to confirm sync against the remote —" +echo " do not rely on a possibly-stale local origin ref." +echo "───────────────────────────────────────────────────────────────────────" + +exit 0 diff --git a/.claude/settings.json b/.claude/settings.json new file mode 100644 index 00000000..128fc56e --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,14 @@ +{ + "hooks": { + "SessionStart": [ + { + "hooks": [ + { + "type": "command", + "command": "$CLAUDE_PROJECT_DIR/.claude/hooks/init-sync-check.sh" + } + ] + } + ] + } +} diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b3a6f059..a3389382 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -14,35 +14,72 @@ permissions: jobs: build-and-test: - name: Build and test + name: Build and test (py${{ matrix.python-version }}) runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + # Matches the classifiers (3.10–3.12). `numpy>=2.0,<3` in + # requirements.txt lets the resolver pick a 3.10-compatible numpy (2.3 + # dropped 3.10). fail-fast:false so a surprise 3.10 wheel gap doesn't + # mask the 3.11/3.12 results. + python-version: ["3.10", "3.11", "3.12"] steps: - uses: actions/checkout@v4 - - name: Set up Python 3.12 + - name: Set up Python ${{ matrix.python-version }} uses: actions/setup-python@v5 with: - python-version: "3.12" + python-version: ${{ matrix.python-version }} cache: pip - name: Install dependencies - # Mirrors the CircleCI "Install Dependencies" step: install runtime + - # test deps from requirements.txt, then numpy (which requirements.txt - # omits — CircleCI pinned it separately). + # requirements.txt now includes numpy (was an ad-hoc `pip install + # numpy==2.3.0` step here). pypowsybl2grid is deprecated & no longer + # listed (its numpy==1.26.4 pin was unsatisfiable against numpy>=2.0.0). run: | python -m pip install --upgrade pip pip install -r requirements.txt - pip install numpy==2.3.0 - - - name: Apply pypowsybl2grid patch - # Patches the installed pypowsybl2grid backend to fix zero-value - # handling (see CLAUDE.md gotcha #6). Must run before the tests import - # the backend. --debug prints the file-content analysis. - run: python scripts/patch_pypowsybl2grid_file.py --debug - name: Run unit tests (exclude slow) run: pytest -m "not slow" + grid2op-optional: + name: grid2op-optional contract (pypowsybl-only) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - name: Set up Python 3.12 + uses: actions/setup-python@v5 + with: + python-version: "3.12" + cache: pip + + - name: Install base package, then remove the grid2op family + # expertop4grid pulls grid2op + lightsim2grid transitively, so a base + # install is NOT grid2op-free; remove the grid2op family to prove the + # pure-pypowsybl path imports and runs without it (enforces the PR #26 + # optionality contract). + run: | + python -m pip install --upgrade pip + pip install -e . + pip uninstall -y grid2op lightsim2grid || true + + - name: Assert grid2op is absent and the pypowsybl path imports without it + run: | + python - <<'PY' + import importlib.util, sys + assert importlib.util.find_spec("grid2op") is None, "grid2op should be uninstalled in this leg" + import expert_op4grid_recommender # package import must not need grid2op + from expert_op4grid_recommender.pipeline import run_analysis, run_analysis_step1 # noqa: F401 + from expert_op4grid_recommender.backends import make_backend, Backend + b = make_backend(Backend.PYPOWSYBL) + assert type(b).__name__ == "PypowsyblBackend", type(b).__name__ + assert "grid2op" not in sys.modules, "importing the pypowsybl path must not import grid2op" + print("grid2op-optional contract OK") + PY + quality: name: Static analysis # Static analysis only (ruff / interrogate / radon parse the code, they do diff --git a/.gitignore b/.gitignore index 12dfcca2..c0982f6e 100644 --- a/.gitignore +++ b/.gitignore @@ -34,3 +34,7 @@ venv*/ # artefacts non versionnés sur le Space (profiling, slides) *.prof *.pptx + +# Runtime-generated overflow graphs (PDF/HTML/.dot written during an analysis; +# e.g. by scripts/benchmark_pipeline.py or an app run) — never a deliverable. +/Overflow_Graph/ diff --git a/CHANGELOG.md b/CHANGELOG.md index 698690df..b418aa82 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,85 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.3.0] - 2026-07-07 + +Follow-up slice of the 2026-07 full code review (findings M3–M6, M8, P3–P4, +and the R7 IHM promotion), on top of 0.2.9. Behaviour-preserving for the +analysis pipeline; the maneuver IHM gains a production serving path and the +dependency floors are reconciled. See `docs/release-notes/v0.3.0.md`. + +### Added + +- **Vendored, import-time pypowsybl integer-value patch (M5).** + `expert_op4grid_recommender/patched_backend.py` applies the + `update_integer_value` `0 → −1` fix on `pypowsybl.grid2op.Backend` via an + idempotent, version-guarded class patch at package import + (`apply_pypowsybl_integer_value_patch()`). No site-packages edit is required; + the standalone `scripts/patch_pypowsybl2grid_file.py` remains only as a manual + fallback. It is a no-op when pypowsybl is absent and self-disables if a future + upstream ships the fix. New coverage: `tests/test_patched_backend.py`. (The + `make_patched_pypowsybl_backend` factory depends on `pypowsybl2grid`, now + deprecated — see below.) +- **Maneuver IHM as a serveable app (R7 partial / M8).** `scripts/manoeuvre_ihm.py` + gains a `create_app()` application factory, a production `waitress` serving + path, and a `/healthz` endpoint, so the Flask IHM can be launched as a proper + WSGI app rather than only via the debug server. + +### Changed + +- **Observability: stop hijacking the root logger (M6).** The package no longer + reconfigures the root logger on import; it logs through its own named logger, + so importing `expert_op4grid_recommender` no longer changes a host + application's logging. `utils/action_rebuilder.py` now re-raises on failure + instead of swallowing the error and returning a partial result. +- **Test infrastructure hardening (M3).** `--strict-markers` plus a registered + `slow` marker (an unregistered mark now errors at collection instead of + silently running as a fast test), opt-in coverage config + (`pytest --cov=expert_op4grid_recommender`), and repair of two dead/broken + tests — including the `get_maintenance_timestep` mock test, which had never + run because it shared a name with the `@slow` real-env test that shadowed it. +- **Dependency declarations synced + grid2op-optional contract enforced (M4).** + `pyproject.toml` / `requirements.txt` floors reconciled (`expertop4grid >= 0.3.2`, + explicit `pydantic` / `pydantic-settings`), the Grid2Op backend's direct deps + moved to a `[grid2op]` extra, and a CI leg that removes the grid2op family and + asserts the pure-pypowsybl pipeline still imports/runs (the PR #26 optionality + contract). + +### Deprecated / Removed + +- **`pypowsybl2grid` deprecated and dropped as a dependency.** Its `0.3.0` wheel + pins `numpy==1.26.4`, which is unsatisfiable against this package's + `numpy>=2.0.0` and broke the install resolve (in CI and anywhere numpy 2 is + required). It is only used by the legacy grid2op+pypowsybl "assistant env" + bridge (`utils/make_assistant_env.create_pypowsybl_backend` → + `patched_backend.make_patched_pypowsybl_backend`); the pure-pypowsybl and + grid2op(lightsim2grid) analysis paths never needed it. It is removed from + `requirements.txt` and `pyproject.toml`, the CI `pypowsybl2grid` patch step is + dropped, the import-time version guard in `utils/make_env_utils.py` is softened + to a `DeprecationWarning`, and the two bridge entry points now emit a + `DeprecationWarning`. Install `pypowsybl2grid` manually into a `numpy<2` + environment if you still need that legacy path. The generic + `apply_pypowsybl_integer_value_patch()` (on `pypowsybl.grid2op.Backend`) is + unaffected. + +### Performance + +- **NetworkManager name-array caching + `set_thermal_limit` O(n²) fix (P4).** + The `name_line` / `name_sub` / … arrays are cached instead of rebuilt on every + property access, and `set_thermal_limit` no longer rescans all branches per + element. +- **Cached membership sets in action application (P3).** Repeated per-element + `in` scans over full element lists are replaced by precomputed sets in the + pypowsybl action-application path. + +### Development + +- **SessionStart git-sync init check.** `.claude/hooks/init-sync-check.sh` + (+ `.claude/settings.json`) is a read-only, non-fatal check that, when a + session starts on the `marota` fork, fetches and reports how far the branch is + from upstream and reminds that PRs target `ainetus`. Runtime-generated + `Overflow_Graph/` artifacts are now gitignored. + ## [0.2.9] - 2026-07-06 ### Changed diff --git a/CLAUDE.md b/CLAUDE.md index bb4c50bb..349807d6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -85,6 +85,9 @@ expert_op4grid_recommender/ │ # re-exported as module attrs for back-compat) ├── config_basic.py # One alternative config variant (Settings(...) with overrides) ├── exceptions.py # Domain exceptions (LoadFlowDivergedError) +├── patched_backend.py # Import-time, version-guarded fix for pypowsybl's +│ # grid2op backend update_integer_value (0→−1); +│ # replaces the site-packages patch script (M5) ├── environment.py # Grid2Op environment setup ├── environment_pypowsybl.py # Pure pypowsybl environment setup ├── data_loader.py # Load action dictionaries from JSON @@ -479,19 +482,44 @@ pytest tests/test_ActionClassifier.py::test_specific # Single test **Core:** - `numpy >= 2.0.0`, `scipy >= 1.13.0`, `pandas`, `networkx` -- `pypowsybl >= 1.13.0`, `pypowsybl2grid >= 0.2.1` -- `expertop4grid >= 0.2.8` (contains alphaDeesp) -- `matplotlib >= 3.8.0` - -**Test:** -- `pytest` +- `pypowsybl >= 1.13.0` (`pypowsybl2grid` is **deprecated** and no longer a + dependency as of 0.3.0 — its `numpy==1.26.4` pin conflicts with `numpy>=2.0.0`; + it is only needed by the legacy grid2op+pypowsybl assistant-env bridge, install + it manually in a `numpy<2` env if required) +- `expertop4grid >= 0.3.2` (contains alphaDeesp; pulls grid2op + lightsim2grid + transitively — so a base install is not grid2op-free at the wheel level) +- `matplotlib >= 3.8.0`, `pydantic >= 2.0`, `pydantic-settings >= 2.0` + +**Extras:** +- `[grid2op]` — `grid2op >= 1.12.1`, `LightSim2Grid >= 0.10.3` (the Grid2Op + backend's *direct* deps, made explicit). `requirements.txt` pins the whole + set for reproducible CI; the CI `grid2op-optional` leg installs the base + package, removes the grid2op family, and asserts the pure-pypowsybl pipeline + still imports/runs (the PR #26 optionality contract). +- `[test]` — `pytest`; `[quality]` — ruff / interrogate / radon / vulture; + `[ihm]` — flask. --- ## Current Development Status -**Current version**: `0.2.9` (see `CHANGELOG.md` for full history) - +**Current version**: `0.3.0` (see `CHANGELOG.md` for full history) + +> **v0.3.0 highlights** (review follow-through on top of 0.2.9 — findings M3–M6, M8, +> P3–P4, R7-partial): the maneuver IHM (`scripts/manoeuvre_ihm.py`) becomes a serveable +> app (`create_app()` factory + `waitress` + `/healthz`, M8/R7); the pypowsybl +> integer-value `0 → −1` fix is a vendored, import-time, version-guarded class patch +> (`patched_backend.py`, M5 — no site-packages edit); the package stops reconfiguring the +> **root** logger and `action_rebuilder` re-raises instead of swallowing (M6); test infra +> hardens with `--strict-markers` + a registered `slow` marker + opt-in coverage, and two +> dead/broken tests are repaired (M3); dependency floors reconcile with a `[grid2op]` extra +> and a grid2op-optional CI leg (M4); and `NetworkManager` name-array caching + a +> `set_thermal_limit` O(n²) fix (P4) plus cached membership sets in action application (P3) +> trim the pypowsybl hot path. **`pypowsybl2grid` is deprecated and dropped as a +> dependency** — its `numpy==1.26.4` pin was unsatisfiable against `numpy>=2.0.0`; only the +> legacy grid2op+pypowsybl assistant-env bridge needs it (install manually in a `numpy<2` +> env). Behaviour-preserving for the analysis pipeline. See `docs/release-notes/v0.3.0.md`. +> > **v0.2.9 highlights** (deep revisions R5 + A5 + R6-partial from the 2026-07 review, plus a > container-aware reassessment fix): discovery restructured around data — one typed > `FamilyResult` per family in `self.results` via a declarative `FAMILY_SPECS` registry (with @@ -678,11 +706,30 @@ combined = action1 + action2 5. **alphaDeesp dependency**: `expertop4grid >= 0.2.8` is required for `AlphaDeesp_warmStart`. -6. **pypowsybl2grid backend patch**: The `PyPowSyBlBackend.update_integer_value` method has an issue with zero-value handling. The original code `changed[value == 0] = False` must be replaced with `value[value == 0] = -1`. **This patch must be applied to the installed package file before running tests**: - ```bash - python scripts/patch_pypowsybl2grid_file.py - ``` - In CI, this is done automatically in the `build-and-test` job of `.github/workflows/ci.yml`. Runtime monkey-patching doesn't work because modules are imported before conftest.py runs. +6. **pypowsybl grid2op backend integer-value fix (0 → −1)**: the buggy method + is `update_integer_value` on **`pypowsybl.grid2op.Backend`** (the internal + delegate `pypowsybl2grid.PyPowSyBlBackend` instantiates as `self._grid`) — + *not* on `PyPowSyBlBackend` itself. It forwards the grid2op bus array to the + native `_pypowsybl.update_grid2op_integer_value`, but grid2op encodes the + disconnected/unset-bus sentinel as `0` while pypowsybl expects `-1`, so the + fix inserts `value[value == 0] = -1` before the native call. + + As of M5 this is applied **at package import time** by + `expert_op4grid_recommender/__init__.py`, via an idempotent, version-guarded + class patch in `expert_op4grid_recommender/patched_backend.py` + (`apply_pypowsybl_integer_value_patch()`). It is a no-op when pypowsybl is + absent and self-disables if a future upstream already applies the fix. **No + site-packages edit is required.** `scripts/patch_pypowsybl2grid_file.py` + remains only as a manual fallback; the `.github/workflows/ci.yml` step that + ran it was removed in 0.3.0. + + **`pypowsybl2grid` deprecated (0.3.0):** its wheel pins `numpy==1.26.4`, + unsatisfiable against `numpy>=2.0.0`, so it is dropped from the declared + dependencies. The generic patch above (on `pypowsybl.grid2op.Backend`) is + unaffected; only the `make_patched_pypowsybl_backend` factory + the + `make_assistant_env.create_pypowsybl_backend` bridge need it, and both now + emit a `DeprecationWarning`. Install `pypowsybl2grid` manually in a `numpy<2` + environment if you still use that legacy grid2op+pypowsybl path. --- diff --git a/Dockerfile b/Dockerfile index 839892d4..17010f61 100644 --- a/Dockerfile +++ b/Dockerfile @@ -34,6 +34,7 @@ WORKDIR /home/user/app RUN pip install --no-cache-dir --upgrade pip \ && pip install --no-cache-dir \ flask \ + "waitress>=3.0" \ "pypowsybl>=1.13.0" \ networkx \ pandas diff --git a/docs/release-notes/v0.3.0.md b/docs/release-notes/v0.3.0.md new file mode 100644 index 00000000..deee6395 --- /dev/null +++ b/docs/release-notes/v0.3.0.md @@ -0,0 +1,125 @@ +# v0.3.0 — Review follow-through: test/observability/dependency hardening, IHM serving path, hot-path perf + +A hardening + small-feature release on top of `0.2.9`, landing the remaining +tractable items from the 2026-07 full-repository review +(`docs/reviews/2026-07_full_code_review.md`): **M3** (test infrastructure), +**M4** (dependency declarations + grid2op-optional CI contract), **M5** +(vendored, import-time pypowsybl backend patch), **M6** (observability), +**M8 / R7-partial** (the maneuver IHM becomes a serveable app), and **P3 / P4** +(pypowsybl hot-path caching). + +Behaviour-preserving for the analysis pipeline: `run_analysis(...)` returns the +same `AnalysisResult` shape and `action_scores` is unchanged. + +## Highlights + +### Maneuver IHM as a serveable app (R7 partial / M8) + +`scripts/manoeuvre_ihm.py` gains a `create_app()` application factory, a +production serving path via `waitress`, and a `/healthz` endpoint. The Flask IHM +can now be launched as a proper WSGI app rather than only through the debug +server, which is what M8 ("maneuver structural debt") and the R7 IHM-promotion +item called for. + +### Vendored, import-time pypowsybl integer-value patch (M5) + +The `update_integer_value` `0 → −1` fix on `pypowsybl.grid2op.Backend` is now +applied by an idempotent, version-guarded **class patch at package import** +(`expert_op4grid_recommender/patched_backend.py` — +`apply_pypowsybl_integer_value_patch()`), wired from `__init__.py`. +Consequences: + +- **No site-packages edit is required.** The standalone + `scripts/patch_pypowsybl2grid_file.py` remains only as a manual fallback. +- It is a **no-op when pypowsybl is absent** and **self-disables** if a future + upstream already ships the fix. + +New coverage: `tests/test_patched_backend.py`. (The +`make_patched_pypowsybl_backend` factory depends on `pypowsybl2grid`, which this +release **deprecates** — see below.) + +### `pypowsybl2grid` deprecated and dropped as a dependency + +`pypowsybl2grid`'s `0.3.0` wheel pins `numpy==1.26.4`, which is unsatisfiable +against this package's `numpy>=2.0.0` — it broke the install resolve (in CI and +anywhere numpy 2 is required). It is only used by the **legacy grid2op+pypowsybl +"assistant env" bridge** (`utils/make_assistant_env.create_pypowsybl_backend` → +`patched_backend.make_patched_pypowsybl_backend`); the pure-pypowsybl and +grid2op(lightsim2grid) analysis paths never needed it. So: + +- Removed from `requirements.txt` and `pyproject.toml` (base deps and the + `[grid2op]` extra). +- The CI `pypowsybl2grid` patch step is dropped. +- The import-time version guard in `utils/make_env_utils.py` is softened from a + hard `RuntimeError` to a `DeprecationWarning`, and the two bridge entry points + now emit a `DeprecationWarning`. + +Install `pypowsybl2grid` manually into a `numpy<2` environment if you still need +that legacy path. The generic `apply_pypowsybl_integer_value_patch()` (on +`pypowsybl.grid2op.Backend`) is unaffected. + +### Observability: stop hijacking the root logger (M6) + +The package no longer reconfigures the **root** logger on import; it logs +through its own named logger, so importing `expert_op4grid_recommender` no +longer changes a host application's logging configuration. +`utils/action_rebuilder.py` now **re-raises** on failure instead of swallowing +the error and returning a partial result. + +### Test infrastructure hardening (M3) + +- `--strict-markers` plus a registered `slow` marker: an unregistered mark + (e.g. a typo `@pytest.mark.slwo`) now **errors at collection** instead of + silently running as a fast test. +- Opt-in coverage config (`pytest --cov=expert_op4grid_recommender`) so the + suite still runs where `pytest-cov` is absent; CI passes the flag. +- Repaired two dead/broken tests — notably the `get_maintenance_timestep` mock + test, which had **never run** because it shared a name with the `@slow` + real-env test that shadowed it (and, once un-shadowed, exposed two latent + assertion bugs, both fixed). + +### Dependency declarations synced + grid2op-optional contract enforced (M4) + +- `pyproject.toml` / `requirements.txt` floors reconciled + (`expertop4grid >= 0.3.2`, explicit `pydantic` / `pydantic-settings`). +- The Grid2Op backend's direct deps moved to a `[grid2op]` extra. +- A CI leg removes the grid2op family and asserts the **pure-pypowsybl pipeline + still imports and runs** (the PR #26 optionality contract). + +### Hot-path performance (P3 / P4) + +- **P4 — `NetworkManager` name-array caching + `set_thermal_limit` O(n²) fix.** + The `name_line` / `name_sub` / … arrays are cached instead of rebuilt on + every property access, and `set_thermal_limit` no longer rescans all branches + per element. +- **P3 — cached membership sets in action application.** Repeated per-element + `in` scans over full element lists in the pypowsybl action-application path + are replaced by precomputed sets. + +## Added + +- `expert_op4grid_recommender/patched_backend.py`; `tests/test_patched_backend.py`. +- `scripts/manoeuvre_ihm.py`: `create_app()` factory, `waitress` serving path, + `/healthz` endpoint. +- A SessionStart git-sync init check (`.claude/hooks/init-sync-check.sh` + + `.claude/settings.json`) — read-only, non-fatal; flags when a `marota`-fork + branch is behind its `ainetus` upstream, and reminds that PRs target + `ainetus`. Runtime-generated `Overflow_Graph/` artifacts are now gitignored. + +## Upgrade notes + +No breaking API changes. The pipeline output is unchanged. Two behavioural +notes for embedders: + +- **Logging** — the package no longer touches the root logger. If you relied on + importing it to configure root logging, configure logging yourself. +- **`action_rebuilder`** — a rebuild failure now raises instead of returning a + partial result; wrap the call if you previously depended on the swallow. +- **`pypowsybl2grid`** — deprecated and no longer installed. If you use the + legacy grid2op+pypowsybl assistant-env bridge + (`make_assistant_env.create_pypowsybl_backend`), `pip install pypowsybl2grid` + yourself in a `numpy<2` environment; otherwise no action is needed. + +Dependency floor moved up (`expertop4grid >= 0.3.2`); Grid2Op-backend deps now +live under the `[grid2op]` extra +(`pip install expert_op4grid_recommender[grid2op]`). diff --git a/expert_op4grid_recommender/__init__.py b/expert_op4grid_recommender/__init__.py index 54dbd037..cf392ac7 100644 --- a/expert_op4grid_recommender/__init__.py +++ b/expert_op4grid_recommender/__init__.py @@ -15,7 +15,7 @@ import logging -__version__ = "0.2.9" +__version__ = "0.3.0" _logger = logging.getLogger(__name__) @@ -52,3 +52,16 @@ def _patched_get_shunt_setpoint(self): # grid2op present but unexpected API surface — log and move on. _logger.debug("Failed to patch grid2op get_shunt_setpoint: %s", exc) # ----------------------------------------------------------- + +# --- pypowsybl grid2op backend integer-value fix (0 sentinel -> -1), M5 --- +# Replaces the historical site-packages edit (scripts/patch_pypowsybl2grid_file.py) +# with an import-time, idempotent, version-guarded class patch. No-op when +# pypowsybl is not installed. Runs before any backend construction. +try: + from expert_op4grid_recommender.patched_backend import ( + apply_pypowsybl_integer_value_patch, + ) + apply_pypowsybl_integer_value_patch() +except Exception as exc: # never let a patch failure break package import + _logger.debug("Could not apply pypowsybl integer-value patch at import: %s", exc) +# ----------------------------------------------------------- diff --git a/expert_op4grid_recommender/cli.py b/expert_op4grid_recommender/cli.py index 0c2aa8ff..c33cf0a5 100644 --- a/expert_op4grid_recommender/cli.py +++ b/expert_op4grid_recommender/cli.py @@ -92,11 +92,17 @@ def main(): else: do_from_scratch = True - dict_action = run_rebuild_actions(n_grid, do_from_scratch, args.repas_file, - dict_action_to_filter_on=dict_action, - voltage_filter_threshold=args.voltage_threshold, - output_file_base_name="reduced_model_actions", - pypowsybl_format=args.pypowsybl_format) + try: + dict_action = run_rebuild_actions(n_grid, do_from_scratch, args.repas_file, + dict_action_to_filter_on=dict_action, + voltage_filter_threshold=args.voltage_threshold, + output_file_base_name="reduced_model_actions", + pypowsybl_format=args.pypowsybl_format) + except Exception as exc: # print acceptable in the CLI entry point + # run_rebuild_actions now RE-RAISES on failure (M6); surface + # it and stop instead of printing "complete" over a failure. + print(f"Action rebuilding failed: {exc}", file=sys.stderr) + sys.exit(1) print("Action rebuilding process complete. Stopping analysis as requested.") return diff --git a/expert_op4grid_recommender/manoeuvre/CLAUDE.md b/expert_op4grid_recommender/manoeuvre/CLAUDE.md index 50848842..d05e687e 100644 --- a/expert_op4grid_recommender/manoeuvre/CLAUDE.md +++ b/expert_op4grid_recommender/manoeuvre/CLAUDE.md @@ -75,8 +75,16 @@ python scripts/render_carrip3_sld.py --grid path/to/grid.xiidm # choisir un poste, modifier DJ/SA, valider/sauver la cible, calculer + # animer la sequence, sauvegarder scenarios et sequences. Doc complete : # docs/manoeuvre/ihm.md +# +# R7 (partiel) : le script expose une fabrique create_app(config) + serve() +# (waitress, threads=1 pour serialiser l'etat pypowsybl partage ; repli sur +# le serveur Flask de dev) et une route /healthz. La promotion physique dans +# un package manoeuvre/ihm/ (blueprints, decoupage de Session, shim) et les +# points M8 restent a faire (differes : la verification exige les 12 tests +# IHM sous pypowsybl). Cf. docs/reviews/2026-07_full_code_review.md (R7/M8). pip install -e ".[ihm]" # guillemets requis sous zsh (sinon: pip install flask) python scripts/manoeuvre_ihm.py --grid path/to/grid.xiidm # http://localhost:8000 +curl http://localhost:8000/healthz # sonde liveness/readiness # Mode dataset RTE 7000 (sans --grid) : source les situations par date/heure # dans le dataset HF OpenSynth/D-GITT-RTE7000-* (telechargement a la demande). diff --git a/expert_op4grid_recommender/patched_backend.py b/expert_op4grid_recommender/patched_backend.py new file mode 100644 index 00000000..46330e9a --- /dev/null +++ b/expert_op4grid_recommender/patched_backend.py @@ -0,0 +1,156 @@ +# Copyright (c) 2025-2026, RTE (https://www.rte-france.com) +# See AUTHORS.txt +# This Source Code Form is subject to the terms of the Mozilla Public License, version 2.0. +# If a copy of the MPL was not distributed with this file, +# you can obtain one at http://mozilla.org/MPL/2.0/. +# SPDX-License-Identifier: MPL-2.0 +"""Vendored, version-guarded replacement for the site-packages patch of +pypowsybl's grid2op backend ``update_integer_value`` (review finding M5). + +Background +---------- +``pypowsybl2grid.PyPowSyBlBackend`` delegates topology writes to an internal +``pypowsybl.grid2op.Backend`` (``pypowsybl/grid2op/impl/backend.py``); it is +that class — NOT ``PyPowSyBlBackend`` itself — whose ``update_integer_value`` +forwards the grid2op bus array straight to the native +``_pypowsybl.update_grid2op_integer_value``. grid2op encodes the +"disconnected / unset" bus sentinel as ``0`` while pypowsybl expects ``-1``, so +the raw forward corrupts topology. + +The historical fix (``scripts/patch_pypowsybl2grid_file.py``) edited the +installed third-party file in place — load-bearing, unguarded against upstream +drift, and mutating a shared venv. This module applies the *same* one-line +correction (``value[value == 0] = -1`` before the native call) at import time +instead: no site-packages file is touched, the wrap is idempotent, and a +best-effort version guard warns if the upstream body ever changes. + +The package's ``__init__.py`` already patches a third-party class at import +time (``grid2op.Backend.get_shunt_setpoint``); this mirrors that precedent. +""" +from __future__ import annotations + +import logging + +_logger = logging.getLogger(__name__) + +# Sentinel attribute stamped on the patched class so we never double-wrap. +_PATCH_FLAG = "_eo4g_integer_value_patched" + +# The upstream body we assume (normalized): a single native passthrough call. +_ASSUMED_CALL = "update_grid2op_integer_value" + + +def _resolve_pp_grid2op_backend_cls(): + """Return the ``pypowsybl.grid2op.Backend`` class, or ``None`` if absent. + + This is exactly the class ``pypowsybl2grid`` instantiates internally as + ``self._grid`` and dispatches ``update_integer_value`` on. + """ + try: + import pypowsybl as pp + return pp.grid2op.Backend + except Exception as exc: # ImportError, or a partial/namespace install + _logger.debug( + "pypowsybl grid2op backend unavailable, skip integer-value patch: %s", exc + ) + return None + + +def _upstream_body_matches_assumption(cls) -> bool: + """Best-effort version guard. + + ``True`` iff the upstream ``update_integer_value`` is still the single native + passthrough we assume (and is not already applying the 0->-1 fix itself). + Returns ``False`` when the source can't be read (namespace/zip installs) so + the caller warns rather than trusting an unverified body. + """ + import inspect + try: + src = inspect.getsource(cls.update_integer_value) + except (OSError, TypeError) as exc: + _logger.debug("Cannot read upstream update_integer_value source: %s", exc) + return False + normalized = " ".join(src.split()) + already_fixed = "value[value == 0]" in normalized or "value[value==0]" in normalized + single_call = normalized.count(_ASSUMED_CALL) == 1 + return single_call and not already_fixed + + +def apply_pypowsybl_integer_value_patch() -> bool: + """Idempotently wrap ``pypowsybl.grid2op.Backend.update_integer_value``. + + Rewrites the grid2op ``0`` bus-sentinel to ``-1`` before the native call. + Returns ``True`` if the patch is in force afterwards, ``False`` if pypowsybl + is unavailable. Safe to call any number of times, and must run before any + backend topology write (i.e. before backend construction). + """ + cls = _resolve_pp_grid2op_backend_cls() + if cls is None: + return False + if getattr(cls, _PATCH_FLAG, False): + return True # already patched by us + + if not _upstream_body_matches_assumption(cls): + try: + import pypowsybl as pp + _ver = getattr(pp, "__version__", "?") + except Exception: + _ver = "?" + _logger.warning( + "pypowsybl %s update_integer_value body differs from the assumed single " + "passthrough (or already applies the 0->-1 fix). Applying the conversion " + "anyway (idempotent), but review patched_backend.py " + "against the installed pypowsybl before trusting results.", + _ver, + ) + + _orig = cls.update_integer_value + + def _patched(self, value_type, value, changed): + # grid2op 0-sentinel (disconnected / unset bus) -> pypowsybl -1. + # Idempotent: after this no zeros remain, so re-applying is a no-op. + value[value == 0] = -1 + return _orig(self, value_type, value, changed) + + _patched.__name__ = "update_integer_value" + _patched.__qualname__ = f"{cls.__qualname__}.update_integer_value" + cls.update_integer_value = _patched + setattr(cls, _PATCH_FLAG, True) + _logger.debug("Applied pypowsybl update_integer_value 0->-1 patch to %s", cls) + return True + + +def _load_pypowsybl2grid_backend_cls(): + """Lazily resolve ``pypowsybl2grid.PyPowSyBlBackend`` (or ``None``).""" + try: + from pypowsybl2grid import PyPowSyBlBackend + return PyPowSyBlBackend + except Exception as exc: # ImportError or partial install + _logger.debug("pypowsybl2grid unavailable: %s", exc) + return None + + +def make_patched_pypowsybl_backend(*args, **kwargs): + """Construct a ``pypowsybl2grid.PyPowSyBlBackend`` with the integer-value + patch guaranteed in force first. + + This is the factory entry point (the review's "importable / testable" + surface). The buggy method lives on the internal ``pypowsybl.grid2op.Backend`` + delegate, so there is nothing useful to override on ``PyPowSyBlBackend`` + itself — instead we guarantee the process-wide class patch before building + the backend. Raises ``ImportError`` if pypowsybl2grid is not installed. + + .. deprecated:: 0.3.0 + ``pypowsybl2grid`` is deprecated and no longer a dependency (its + ``numpy==1.26.4`` pin conflicts with ``numpy>=2.0.0``). Install it + manually into a ``numpy<2`` environment if you still need this factory. + The generic ``apply_pypowsybl_integer_value_patch()`` remains active. + """ + backend_cls = _load_pypowsybl2grid_backend_cls() + if backend_cls is None: + raise ImportError( + "pypowsybl2grid is deprecated and no longer a dependency; install it " + "manually in a numpy<2 environment to build a PyPowSyBlBackend." + ) + apply_pypowsybl_integer_value_patch() + return backend_cls(*args, **kwargs) diff --git a/expert_op4grid_recommender/pypowsybl_backend/action_space.py b/expert_op4grid_recommender/pypowsybl_backend/action_space.py index b3005270..1eed11c6 100644 --- a/expert_op4grid_recommender/pypowsybl_backend/action_space.py +++ b/expert_op4grid_recommender/pypowsybl_backend/action_space.py @@ -67,16 +67,22 @@ def __init__(self, switch_states: Dict[str, bool]): def apply_switch_changes(nm: 'NetworkManager'): net = nm.network - + + # P3: fetch the switch index ONCE per apply (was 1-2 full ~85k-row + # table fetches per switch id). The set of switches is variant- + # invariant — only their `open` column changes — so a single read at + # the top of apply is correct regardless of the active variant. + switch_ids = set(net.get_switches().index) + # Helper to find actual switch ID if prefixed def get_actual_sid(sid): - if sid in net.get_switches().index: + if sid in switch_ids: return sid # Try with underscores replaced or prefix removed # Example: PYMONP3_PYMON3COUPL -> PYMON3COUPL if '_' in sid: parts = sid.split('_', 1) - if parts[1] in net.get_switches().index: + if parts[1] in switch_ids: return parts[1] return None diff --git a/expert_op4grid_recommender/pypowsybl_backend/network_manager.py b/expert_op4grid_recommender/pypowsybl_backend/network_manager.py index 3534ccf3..80eb40f5 100644 --- a/expert_op4grid_recommender/pypowsybl_backend/network_manager.py +++ b/expert_op4grid_recommender/pypowsybl_backend/network_manager.py @@ -220,6 +220,34 @@ def _cache_element_info(self): # PSTs - cache phase tap changers self._cache_pst_info() + # P4: materialize the name arrays ONCE. The backing id lists + # (_line_ids / _substation_ids / _gen_ids / _load_ids) are structural + # (element identity + order) and never change across variants or during + # this NetworkManager's lifetime — a network reload builds a fresh + # manager — so the arrays are safe to cache. Without this, the name_* + # properties rebuilt a fresh Tuple[np.ndarray, np.ndarray]: @property def name_line(self) -> np.ndarray: - """Array of line names (compatible with grid2op interface).""" - return np.array(self._line_ids) - + """Array of line names (compatible with grid2op interface). Cached read-only (P4).""" + return self._cached_name_line + @property def name_sub(self) -> np.ndarray: - """Array of substation names (voltage level IDs).""" - return np.array(self._substation_ids) - + """Array of substation names (voltage level IDs). Cached read-only (P4).""" + return self._cached_name_sub + @property def name_gen(self) -> np.ndarray: - """Array of generator names.""" - return np.array(self._gen_ids) + """Array of generator names. Cached read-only (P4).""" + return self._cached_name_gen @property def gen_energy_source(self) -> np.ndarray: """Array of generator energy source strings (e.g. 'WIND', 'SOLAR', 'THERMAL', ...).""" return self._gen_energy_source.copy() - + @property def name_load(self) -> np.ndarray: - """Array of load names.""" - return np.array(self._load_ids) + """Array of load names. Cached read-only (P4).""" + return self._cached_name_load @property def n_line(self) -> int: @@ -533,25 +561,29 @@ def run_load_flow(self, dc: Optional[bool] = None, fast: bool = False, def disconnect_line(self, line_id: str): """Disconnect a line (open both terminals).""" - if line_id in self.network.get_lines().index: + # P3: O(1) membership on the cached sets instead of fetching the full + # lines / 2wt tables on every call. The element index is structural and + # never changes across variants, so the sets stay valid. + if line_id in self._lines_set: self.network.update_lines(id=line_id, connected1=False, connected2=False) - elif line_id in self.network.get_2_windings_transformers().index: + elif line_id in self._trafos_set: self.network.update_2_windings_transformers( id=line_id, connected1=False, connected2=False ) - + def reconnect_line(self, line_id: str, bus_or: int = 1, bus_ex: int = 1): """ Reconnect a line to specified buses. - + Args: line_id: Line identifier bus_or: Bus number at origin (1 or 2) bus_ex: Bus number at extremity (1 or 2) """ - if line_id in self.network.get_lines().index: + # P3: cached-set membership instead of full-table fetches (see disconnect_line). + if line_id in self._lines_set: self.network.update_lines(id=line_id, connected1=True, connected2=True) - elif line_id in self.network.get_2_windings_transformers().index: + elif line_id in self._trafos_set: self.network.update_2_windings_transformers( id=line_id, connected1=True, connected2=True ) diff --git a/expert_op4grid_recommender/pypowsybl_backend/simulation_env.py b/expert_op4grid_recommender/pypowsybl_backend/simulation_env.py index 11eccca7..18c01dec 100644 --- a/expert_op4grid_recommender/pypowsybl_backend/simulation_env.py +++ b/expert_op4grid_recommender/pypowsybl_backend/simulation_env.py @@ -147,20 +147,24 @@ def set_thermal_limit(self, thermal_limits: Union[List[float], np.ndarray]): Args: thermal_limits: Array of thermal limits in same order as name_line """ + # Hoist name_line to a local once (P4): even though it is now a cached + # property, keeping a single reference documents the O(n) contract and + # is robust to any future property-side regression. + names = self.name_line for i, limit in enumerate(thermal_limits): - line_name = self.name_line[i] - self._thermal_limits[line_name] = limit - + self._thermal_limits[names[i]] = limit + def get_thermal_limit(self) -> np.ndarray: """ Get thermal limits as array. - + Returns: Array of thermal limits in same order as name_line """ + names = self.name_line return np.array([ - self._thermal_limits.get(ln, 9999.0) - for ln in self.name_line + self._thermal_limits.get(ln, 9999.0) + for ln in names ]) # ========== Properties matching grid2op interface ========== diff --git a/expert_op4grid_recommender/utils/action_rebuilder.py b/expert_op4grid_recommender/utils/action_rebuilder.py index 449c27e2..c5173254 100644 --- a/expert_op4grid_recommender/utils/action_rebuilder.py +++ b/expert_op4grid_recommender/utils/action_rebuilder.py @@ -16,6 +16,7 @@ import copy import json +import logging import os from collections import defaultdict from expert_op4grid_recommender.utils.helpers import Timer @@ -27,6 +28,9 @@ get_all_switch_descriptions, ) +logger = logging.getLogger(__name__) + + def make_description_unitaire(switches_by_voltage_level): """ Creates a unitary description string for a switch action. @@ -508,8 +512,9 @@ def run_rebuild_actions(n_grid, do_from_scratch, repas_file_path, dict_action_to print(f"Successfully rebuilt actions. Saved to: {output_file_path}") return new_dict_actions - except Exception as e: - import traceback - print(f"Error rebuilding actions: {e}") - traceback.print_exc() - return dict_action_to_filter_on # Return original on failure \ No newline at end of file + except Exception: + # M6: log with the full traceback and RE-RAISE. Previously this + # swallowed every error and returned the untouched input dict, so a + # caller could not tell "rebuilt N actions" from "everything failed". + logger.error("Error rebuilding actions from %s", repas_file_path, exc_info=True) + raise \ No newline at end of file diff --git a/expert_op4grid_recommender/utils/make_assistant_env.py b/expert_op4grid_recommender/utils/make_assistant_env.py index 1afb0788..23ee78c1 100644 --- a/expert_op4grid_recommender/utils/make_assistant_env.py +++ b/expert_op4grid_recommender/utils/make_assistant_env.py @@ -13,12 +13,17 @@ make_default_params, create_olf_rte_parameter) +logger = logging.getLogger(__name__) + try: import grid2op from grid2op.Environment import Environment from grid2op.Backend import Backend from grid2op.Chronics import ChangeNothing - from pypowsybl2grid import PyPowSyBlBackend + # Import guard for _HAS_GRID2OP: the backend is built through + # make_patched_pypowsybl_backend (M5), but this import must still fail when + # pypowsybl2grid is absent so _HAS_GRID2OP is set correctly. + from pypowsybl2grid import PyPowSyBlBackend # noqa: F401 import numpy as np _HAS_GRID2OP = True except (ImportError, Exception): @@ -33,15 +38,38 @@ def create_pypowsybl_backend( Silences powsybl/pypowsybl2grid chatter to ``ERROR`` and wires in the RTE OpenLoadFlow parameter set so assistant runs match what the evaluator would do. + + .. deprecated:: 0.3.0 + This path depends on ``pypowsybl2grid``, which is no longer a + dependency (its ``numpy==1.26.4`` pin conflicts with ``numpy>=2.0.0``). + Install ``pypowsybl2grid`` manually into a ``numpy<2`` environment if + you still need the legacy grid2op+pypowsybl assistant env. """ + import warnings + warnings.warn( + "create_pypowsybl_backend() / the grid2op+pypowsybl assistant-env bridge " + "depends on the deprecated pypowsybl2grid package, which is no longer a " + "dependency of expert_op4grid_recommender. Install it manually in a " + "numpy<2 environment if you still rely on this path.", + DeprecationWarning, + stacklevel=2, + ) if not _HAS_GRID2OP: raise ImportError("grid2op and pypowsybl2grid are required for create_pypowsybl_backend()") - logging.basicConfig() - logging.getLogger().setLevel(logging.ERROR) + # Silence powsybl / pypowsybl2grid chatter WITHOUT touching the root logger. + # The previous logging.basicConfig() + root setLevel(ERROR) was a global + # side effect that silenced the host application's own INFO/WARNING logs + # every time an assistant env was built (M6). logging.getLogger('powsybl').setLevel(logging.ERROR) logging.getLogger('pypowsybl2grid').setLevel(logging.ERROR) lf_parameters = create_olf_rte_parameter() - return PyPowSyBlBackend(n_busbar_per_sub=n_busbar_per_sub, + # M5: build through the vendored factory so the pypowsybl grid2op backend's + # integer-value 0->-1 fix is guaranteed in force (no site-packages patch). + from expert_op4grid_recommender.patched_backend import ( + make_patched_pypowsybl_backend, + ) + return make_patched_pypowsybl_backend( + n_busbar_per_sub=n_busbar_per_sub, check_isolated_and_disconnected_injections=check_isolated_and_disconnected_injections, lf_parameters=lf_parameters) @@ -77,7 +105,7 @@ def make_grid2op_assistant_env( param=params ) else: - print("Warning: this is a bare environment with no chronics") + logger.warning("Bare environment with no chronics at %s", path) env = grid2op.make(path, backend=backend, allow_detachment=allow_detachment, diff --git a/expert_op4grid_recommender/utils/make_env_utils.py b/expert_op4grid_recommender/utils/make_env_utils.py index 0f7b671a..a46e7fd5 100644 --- a/expert_op4grid_recommender/utils/make_env_utils.py +++ b/expert_op4grid_recommender/utils/make_env_utils.py @@ -1,11 +1,13 @@ """Shared helpers for constructing Grid2Op / pypowsybl environments. Validates minimum package versions at import time (grid2op, lightsim2grid, -pypowsybl, pypowsybl2grid) and exposes small factories used by -:mod:`make_training_env` and :mod:`make_assistant_env` to build consistent -loader / loadflow / Grid2Op parameter objects. +pypowsybl) and exposes small factories used by :mod:`make_training_env` and +:mod:`make_assistant_env` to build consistent loader / loadflow / Grid2Op +parameter objects. ``pypowsybl2grid`` is deprecated (v0.3.0) and no longer a +dependency; if present it only triggers a :class:`DeprecationWarning`. """ +import warnings from typing import Any, Dict, Literal, Union from packaging import version from importlib.metadata import version as version_importlib,PackageNotFoundError @@ -40,13 +42,22 @@ f"upgrade it from pypi with:\n" f'\t `pip install "LightSim2Grid>={MIN_LS_VERSION}"`') try: + # pypowsybl2grid is DEPRECATED (v0.3.0) and no longer a declared dependency: + # its wheel pins numpy==1.26.4, unsatisfiable against numpy>=2.0.0. It is only + # needed by the legacy grid2op+pypowsybl assistant-env bridge. If a user has + # installed it manually, warn rather than hard-enforcing a minimum version. _pp2grid_version = version_importlib("pypowsybl2grid") - if version.parse(_pp2grid_version) < MIN_PP2GRID_VERSION: - raise RuntimeError(f"pypowsybl2grid minimum version needed: {MIN_PP2GRID_VERSION}. Please " - f"upgrade it from pypi with:\n" - f'\t `pip install "pypowsybl2grid>={MIN_PP2GRID_VERSION}"`') + warnings.warn( + "pypowsybl2grid is deprecated and no longer a dependency of " + "expert_op4grid_recommender (its numpy==1.26.4 pin conflicts with " + "numpy>=2.0.0). It is only used by the legacy grid2op+pypowsybl " + "assistant-env bridge; the pure-pypowsybl and grid2op(lightsim2grid) " + "paths do not need it.", + DeprecationWarning, + stacklevel=2, + ) except ImportError: - pass # pypowsybl2grid is optional (only needed for grid2op backend) + pass # pypowsybl2grid is optional/deprecated (legacy grid2op backend only) # Define the potential pypowsybl package names PACKAGES = ["pypowsybl", "pypowsybl-rte"] diff --git a/pyproject.toml b/pyproject.toml index 27496a93..e1b9330d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "expert_op4grid_recommender" -version = "0.2.9" +version = "0.3.0" authors = [ { name="RTE", email="rte@rte-france.com" }, ] @@ -28,8 +28,14 @@ dependencies = [ "pandas", "networkx", "pypowsybl>=1.13.0", - "pypowsybl2grid>=0.2.1", - "expertop4grid>=0.2.8", + # pypowsybl2grid DEPRECATED and dropped as a dependency (v0.3.0): its 0.3.0 + # wheel pins numpy==1.26.4, which is unsatisfiable against numpy>=2.0.0 and + # broke installs. It is only needed by the legacy grid2op+pypowsybl + # "assistant env" bridge (utils/make_assistant_env.create_pypowsybl_backend); + # install it manually in a numpy<2 environment if you still need that path. + # Floor bumped 0.2.8 -> 0.3.2 to match the tested family (requirements.txt + # pins 0.3.2.post3) and resolve the pyproject/requirements disagreement (M4). + "expertop4grid>=0.3.2", "matplotlib>=3.8.0", "pydantic>=2.0", "pydantic-settings>=2.0", @@ -39,6 +45,19 @@ dependencies = [ test = [ "pytest", ] +# The Grid2Op simulation backend. NOTE: `expertop4grid` (a base dependency) +# already pulls Grid2Op + lightsim2grid transitively, so a base install is not +# grid2op-free at the wheel level today; this extra makes the grid2op backend's +# DIRECT requirements explicit and pins the floors the code enforces +# (utils/make_env_utils.py). The CI `grid2op-optional` leg enforces the +# code-level contract. `pypowsybl2grid` is intentionally NOT declared here or in +# the base deps (deprecated v0.3.0 — its numpy==1.26.4 pin is unsatisfiable); +# install it manually in a numpy<2 env only if you need the legacy +# grid2op+pypowsybl assistant-env bridge. +grid2op = [ + "grid2op>=1.12.1", + "LightSim2Grid>=0.10.3", +] quality = [ "radon>=6.0", "vulture>=2.10", @@ -47,6 +66,10 @@ quality = [ ] ihm = [ "flask>=3.0", + # Production WSGI server for the manoeuvre IHM / HuggingFace Space (R7). + # Served with threads=1 to preserve request serialisation (shared pypowsybl + # network state); falls back to the Flask dev server if absent. + "waitress>=3.0", ] [tool.setuptools.packages.find] @@ -113,20 +136,48 @@ select = ["E4", "E7", "E9", "F"] "tests/test_ActionRuleValidator.py" = ["E402", "E701", "E702"] "tests/test_action_rebuilder.py" = ["E712", "F841"] "tests/test_config_override.py" = ["E712"] -"tests/test_expert_op4grid_analyzer.py" = ["E741", "F403", "F405", "F811", "F841"] +"tests/test_expert_op4grid_analyzer.py" = ["E741", "F403", "F405", "F841"] "tests/test_initial_lf_voltage_init_mode.py" = ["E402", "F841"] -"tests/test_islanding_mw.py" = ["F811"] "tests/test_lazy_action_dict.py" = ["F841"] "tests/test_lf_fallback_non_converged.py" = ["E402"] "tests/test_min_action_counts.py" = ["F841"] "tests/test_pst_actions.py" = ["E712"] "tests/test_pypowsybl_backend.py" = ["E712"] -"tests/test_simulation_optimizations.py" = ["F821"] "tests/test_superposition.py" = ["E701"] "tests/test_superposition_action_types.py" = ["F841"] "tests/test_superposition_extended.py" = ["E402"] "tests/test_superposition_rho_estimation.py" = ["E701"] +[tool.pytest.ini_options] +# Scope collection to the suite and register every custom marker. With +# ``--strict-markers`` an unregistered mark (e.g. a typo ``@pytest.mark.slwo``) +# now ERRORS at collection instead of silently running as a fast test — closing +# the gap called out as M3 in docs/reviews/2026-07_full_code_review.md. +testpaths = ["tests"] +addopts = "--strict-markers" +markers = [ + "slow: end-to-end test needing a full grid2op/pypowsybl environment and real data (deselect with '-m \"not slow\"')", +] + +# Coverage is opt-in (``pytest --cov``) so the suite still runs where pytest-cov +# is absent; CI passes ``--cov=expert_op4grid_recommender``. See M3 / R8. +[tool.coverage.run] +branch = true +source = ["expert_op4grid_recommender"] +omit = [ + "*/tests/*", + # ``manoeuvre`` carries its own docstring/quality gate; measured separately. + "*/manoeuvre/*", +] + +[tool.coverage.report] +show_missing = true +skip_covered = true +# Ratchet floor. Intentionally 0 today: large pypowsybl/grid2op-gated areas are +# not exercised in the default (grid2op-less) CI leg, so a non-zero floor would +# be misleading. Raise this as the grid2op CI leg + new tests land — never lower. +fail_under = 0 + [tool.interrogate] # Couverture des docstrings. Les fonctions imbriquées (closures), __init__ et # méthodes magiques ne sont pas exigées ; les fonctions privées de premier diff --git a/requirements.txt b/requirements.txt index a396117e..c91ab931 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,11 +1,31 @@ +# Pinned, reproducible CI/dev environment. A superset of pyproject.toml's floors +# (the grid2op family is included here because the default CI leg tests the +# grid2op-installed path). Keep floors consistent with pyproject.toml and with +# the runtime guards in utils/make_env_utils.py (the MIN_* constants). +# +# numpy is a real runtime dependency (pyproject lists it); it used to be missing +# here and installed ad-hoc in CI (`pip install numpy==2.3.0`). The `<3` upper +# bound lets the resolver pick a Python-3.10-compatible numpy (2.3 dropped 3.10) +# while keeping 2.3.x on 3.11/3.12. +numpy>=2.0.0,<3 scipy>=1.13.0 -grid2op==1.12.1 -LightSim2Grid==0.10.3 pandas==2.3.3 -networkx==3.5 +# networkx 3.5 dropped Python 3.10 (latest 3.10 wheel is 3.4.2). Use a range so +# the py3.10 matrix leg resolves to 3.4.x while py3.11/3.12 pick 3.5 — same +# reason as the numpy `<3` bound above. +networkx>=3.4,<3.6 pypowsybl==1.14.0 -pypowsybl2grid==0.3.0 +# pypowsybl2grid DEPRECATED and removed as a dependency (v0.3.0): its 0.3.0 +# wheel pins numpy==1.26.4, unsatisfiable against this package's numpy>=2.0.0 +# (it broke the CI resolve). It is only needed by the legacy grid2op+pypowsybl +# "assistant env" bridge; install it manually into a numpy<2 environment if you +# still need create_pypowsybl_backend(). See docs/release-notes/v0.3.0.md. expertop4grid==0.3.2.post3 matplotlib==3.10.7 pydantic>=2.0 pydantic-settings>=2.0 +# --- grid2op simulation backend (also declared as the [grid2op] extra in +# pyproject; installed in the default CI leg, removed in the grid2op-optional +# contract leg) --- +grid2op==1.12.1 +LightSim2Grid==0.10.3 diff --git a/scripts/manoeuvre_ihm.py b/scripts/manoeuvre_ihm.py index 4d22e5f9..40d74f92 100644 --- a/scripts/manoeuvre_ihm.py +++ b/scripts/manoeuvre_ihm.py @@ -2117,6 +2117,59 @@ def api_manual_start(): +@app.get("/healthz") +def healthz(): + """Liveness/readiness probe for containers and the HuggingFace Space. + + Reports whether a network session is loaded and whether the dataset source + is enabled, so an orchestrator can distinguish "process up" from "ready". + """ + return jsonify( + status="ok", + session_loaded=SESSION is not None, + dataset_enabled=bool(DATASET.get("enabled")), + ), 200 + + +def create_app(config=None): + """Application factory (R7). + + Returns the configured Flask ``app``. The IHM is intentionally mono-user + (the pypowsybl network is shared mutable state), so this returns the module + singleton rather than minting a fresh app; ``config`` lets a caller apply + the static wiring ``main()`` used to poke into globals — ``scenarios_dir``, + ``sequences_dir``, ``dataset`` (merged in place), and ``grid`` (loaded into + a fresh :class:`Session`) — without going through argparse. + """ + global SCEN_DIR, SEQ_DIR, SESSION + if config: + if config.get("scenarios_dir"): + SCEN_DIR = pathlib.Path(config["scenarios_dir"]) + if config.get("sequences_dir"): + SEQ_DIR = pathlib.Path(config["sequences_dir"]) + if config.get("dataset"): + DATASET.update(config["dataset"]) # in place → shared object + if config.get("grid"): + SESSION = Session(pp.network.load(config["grid"])) + return app + + +def serve(flask_app, host="127.0.0.1", port=8000): + """Serve the IHM under a production WSGI server when available (R7). + + Uses waitress with ``threads=1`` — the pypowsybl network is shared mutable + state, so requests must stay serialised (the same guarantee the historical + ``app.run(threaded=False)`` gave). Falls back to Flask's dev server if + waitress is not installed. + """ + try: + from waitress import serve as _wserve + _wserve(flask_app, host=host, port=port, threads=1) + except ImportError: + # threaded=False : sérialise les requêtes (l'état réseau pypowsybl est partagé). + flask_app.run(host=host, port=port, threaded=False) + + def main(): global SESSION ap = argparse.ArgumentParser(description=__doc__) @@ -2175,8 +2228,10 @@ def main(): print(f"Mode dataset : {DATASET['repo']} (cache {DATASET['cache_dir']}). " "Choisir une date/heure dans l'IHM.") print(f"IHM Manœuvre : http://{args.host}:{args.port} (Ctrl-C pour arrêter)") - # threaded=False : sérialise les requêtes (l'état réseau pypowsybl est partagé). - app.run(host=args.host, port=args.port, threaded=False) + # create_app() is a no-op here (config already applied above); kept so the + # factory is exercised on the CLI path. serve() prefers waitress (threads=1) + # and falls back to the Flask dev server. + serve(create_app(), host=args.host, port=args.port) if __name__ == "__main__": diff --git a/scripts/patch_pypowsybl2grid_file.py b/scripts/patch_pypowsybl2grid_file.py index 2c60b43c..5d7dd165 100644 --- a/scripts/patch_pypowsybl2grid_file.py +++ b/scripts/patch_pypowsybl2grid_file.py @@ -1,9 +1,19 @@ #!/usr/bin/env python3 """ -Script to patch pypowsybl2grid backend.py file directly. - -This patches the installed pypowsybl2grid package to fix the zero-value handling -issue in update_integer_value method. +Script to patch pypowsybl's grid2op backend.py file directly. + +MANUAL FALLBACK (M5): the zero-value handling fix in +``pypowsybl.grid2op.Backend.update_integer_value`` is normally applied at +package import time by ``expert_op4grid_recommender/patched_backend.py`` (an +idempotent, version-guarded runtime class patch), so this script is no longer +required for tests or CI. It remains as a manual recovery path — e.g. if the +runtime patch's version guard fires against an unfamiliar upstream and an +operator wants to force the in-file edit. The edit and the runtime wrap compose +harmlessly (both are idempotent). + +This patches the installed package to fix the zero-value handling issue in the +update_integer_value method (grid2op encodes the disconnected/unset bus as 0; +pypowsybl expects -1, so value[value == 0] = -1 is inserted before the native call). Usage: python scripts/patch_pypowsybl2grid_file.py [--dry-run] [--revert] [--debug] diff --git a/tests/conftest.py b/tests/conftest.py index 4394c60a..7ddba212 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -70,13 +70,15 @@ # Now import pytest import pytest -# NOTE: pypowsybl2grid backend patch is applied via scripts/patch_pypowsybl2grid_file.py -# This must be run BEFORE tests (see .github/workflows/ci.yml) -# Runtime monkey-patching doesn't work because modules are imported before conftest runs +# NOTE: the pypowsybl grid2op backend integer-value fix (0 sentinel -> -1) is +# applied at package IMPORT time by expert_op4grid_recommender/__init__.py, via +# an idempotent, version-guarded class patch of pypowsybl.grid2op.Backend +# (expert_op4grid_recommender/patched_backend.py, M5). Because +# config_test star-imports config — which imports the package — the patch is in +# force before any backend is constructed; no site-packages edit is required. # -# For local development, run: -# python scripts/patch_pypowsybl2grid_file.py -# before running tests. +# scripts/patch_pypowsybl2grid_file.py remains only as a manual fallback (e.g. if +# the runtime patch's version guard fires against an unfamiliar upstream). @pytest.fixture(scope="session", autouse=True) diff --git a/tests/test_action_rebuilder.py b/tests/test_action_rebuilder.py index d961d7c8..587985f3 100644 --- a/tests/test_action_rebuilder.py +++ b/tests/test_action_rebuilder.py @@ -1379,20 +1379,23 @@ def test_rebuild_mode(self, mock_open, mock_path_join, mock_parse, mock_rebuild, mock_rebuild.assert_called_once() @patch('expert_op4grid_recommender.utils.action_rebuilder.repas.parse_json') - def test_returns_original_on_failure(self, mock_parse, mock_network): - """Test that original dict is returned on failure.""" + def test_raises_on_failure(self, mock_parse, mock_network): + """A failure now RE-RAISES (M6) instead of silently returning the input. + + Previously the function swallowed every exception and returned the + untouched input dict, so callers could not tell success from failure. + """ mock_parse.side_effect = Exception("Parse error") - + original_dict = {"original": {"content": {}}} - - result = run_rebuild_actions( - mock_network, - do_from_scratch=False, - repas_file_path="fake_repas.json", - dict_action_to_filter_on=original_dict - ) - - assert result == original_dict + + with pytest.raises(Exception, match="Parse error"): + run_rebuild_actions( + mock_network, + do_from_scratch=False, + repas_file_path="fake_repas.json", + dict_action_to_filter_on=original_dict + ) # ============================================================================= diff --git a/tests/test_expert_op4grid_analyzer.py b/tests/test_expert_op4grid_analyzer.py index 3ba73245..0823d6d9 100644 --- a/tests/test_expert_op4grid_analyzer.py +++ b/tests/test_expert_op4grid_analyzer.py @@ -159,6 +159,9 @@ def __init__(self, substations_id=None, lines_ex_id=None, lines_or_id=None, line self.lines_or_id = lines_or_id or {} self.lines_status= lines_status or {} self.action_id = str(substations_id)+str(lines_or_id) + # Mirrors the meaningful (non-empty) parts of the action dict passed to + # MockActionSpace; an empty action (e.g. no line to reconnect) has {}. + self.content = {} def get_topological_impact(self): # Return dummy lists, just to satisfy the interface @@ -182,14 +185,20 @@ def __call__(self, action_dict): if "set_bus" in action_dict: set_bus = action_dict["set_bus"] if "substations_id" in set_bus: - return MockActionObject(substations_id=set_bus["substations_id"]) + act = MockActionObject(substations_id=set_bus["substations_id"]) else: # Handles lines_ex_id / lines_or_id for act_defaut - return MockActionObject(lines_ex_id=set_bus.get("lines_ex_id"), + act = MockActionObject(lines_ex_id=set_bus.get("lines_ex_id"), lines_or_id=set_bus.get("lines_or_id")) elif "set_line_status" in action_dict: set_line_status=action_dict["set_line_status"] - return MockActionObject(lines_status=set_line_status) + act = MockActionObject(lines_status=set_line_status) + else: + act = MockActionObject() + # Expose the meaningful (non-empty) parts of the action dict as `.content` + # so an empty action reconstructs to {} (see get_maintenance_timestep tests). + act.content = {key: value for key, value in action_dict.items() if value} + return act def mock_action_space(content): @@ -327,9 +336,13 @@ def __init__(self, name_line, maintenance_array): self.action_space = MockActionSpace() -def test_get_maintenance_timestep(): +def test_get_maintenance_timestep_scenarios(): """ Test get_maintenance_timestep for various reconnection scenarios. + + (Renamed from ``test_get_maintenance_timestep`` — it used to share the + name of the ``@pytest.mark.slow`` real-env test below, so pytest only ever + collected the last definition and this mock-based one never ran.) """ # Setup mock environment @@ -368,9 +381,11 @@ def test_get_maintenance_timestep(): assert reconnected == [] assert act.content == {} - # --- Case 3: timestep where all lines are out of maintenance --- + # --- Case 3: timestep where the maintained line has not been released yet --- + # At t=0 L1 is still in maintenance, so although it was maintained at start it + # is not yet reconnectable — the function must return nothing. act, reconnected = get_maintenance_timestep( - timestep=2, + timestep=0, lines_non_reconnectable=lines_non_reconnectable, env=env, do_reco_maintenance=True @@ -1093,6 +1108,17 @@ def test_reproducibility_bare_env_small_grid_test(): # 'expert_op4grid_recommender.config' actually points to 'tests.config_test' import expert_op4grid_recommender.config as config + # The grid2op (Backend.GRID2OP) path builds its environment through the + # grid2op+pypowsybl "assistant env" bridge (make_grid2op_assistant_env -> + # create_pypowsybl_backend), which requires pypowsybl2grid. That package is + # DEPRECATED and no longer a dependency (its numpy==1.26.4 pin conflicts with + # numpy>=2.0.0), so skip cleanly where it is absent (e.g. CI). Install + # pypowsybl2grid in a numpy<2 env to exercise this test locally. + pytest.importorskip( + "pypowsybl2grid", + reason="Backend.GRID2OP assistant-env bridge needs the deprecated pypowsybl2grid", + ) + # Test parameters test_id = "Case_BareEnvSmallGrid_T0" timestep = 0 diff --git a/tests/test_islanding_mw.py b/tests/test_islanding_mw.py index 94f419c5..e6e50b60 100644 --- a/tests/test_islanding_mw.py +++ b/tests/test_islanding_mw.py @@ -1,11 +1,13 @@ import pytest -import pypowsybl from pathlib import Path -from expert_op4grid_recommender.pypowsybl_backend import NetworkManager, ActionSpace, PypowsyblObservation -# Skip all tests if pypowsybl is not available +# Skip all tests if pypowsybl is not available. This MUST precede the +# pypowsybl-dependent imports below, otherwise collection raises +# ModuleNotFoundError before importorskip can turn it into a skip. pypowsybl = pytest.importorskip("pypowsybl") +from expert_op4grid_recommender.pypowsybl_backend import NetworkManager, ActionSpace, PypowsyblObservation # noqa: E402 + def test_islanding_small_grid_case(): """Test islanding case suggested by user on the small grid.""" grid_path = Path("/home/marotant/dev/Expert_op4grid_recommender/data/bare_env_small_grid_test/grid.xiidm") diff --git a/tests/test_patched_backend.py b/tests/test_patched_backend.py new file mode 100644 index 00000000..bd8c0d27 --- /dev/null +++ b/tests/test_patched_backend.py @@ -0,0 +1,82 @@ +# Copyright (c) 2025-2026, RTE (https://www.rte-france.com) +# See AUTHORS.txt +# This Source Code Form is subject to the terms of the Mozilla Public License, version 2.0. +# If a copy of the MPL was not distributed with this file, +# you can obtain one at http://mozilla.org/MPL/2.0/. +# SPDX-License-Identifier: MPL-2.0 +"""Tests for the vendored pypowsybl integer-value patch (M5). + +Two tiers: a pure-logic check of the 0->-1 correction that needs no pypowsybl, +and pypowsybl-guarded checks of the actual class patch. The latter skip cleanly +where pypowsybl is unavailable. +""" +import numpy as np +import pytest + +from expert_op4grid_recommender import patched_backend as pb + + +def test_zero_to_minus_one_conversion_is_idempotent(): + """The correction itself: grid2op's 0 bus-sentinel -> pypowsybl -1.""" + v = np.array([0, 1, 0, 2, -1], dtype=np.int32) + v[v == 0] = -1 + assert v.tolist() == [-1, 1, -1, 2, -1] + v[v == 0] = -1 # idempotent — no zeros remain + assert v.tolist() == [-1, 1, -1, 2, -1] + + +def test_apply_patch_returns_false_without_pypowsybl(monkeypatch): + """When pypowsybl can't be resolved, applying the patch is a safe no-op.""" + monkeypatch.setattr(pb, "_resolve_pp_grid2op_backend_cls", lambda: None) + assert pb.apply_pypowsybl_integer_value_patch() is False + + +def test_patch_wraps_and_converts_on_a_stub_class(monkeypatch): + """Drive the wrapping logic against a stub 'backend' class — no pypowsybl. + + Proves the wrapper (a) rewrites 0->-1 before delegating, (b) stamps the + idempotency flag, and (c) does not double-wrap on a second apply. + """ + calls = [] + + class _StubBackend: + def update_integer_value(self, value_type, value, changed): + calls.append(value.copy()) + + monkeypatch.setattr(pb, "_resolve_pp_grid2op_backend_cls", lambda: _StubBackend) + # Force the "matches assumption" guard so no warning path is exercised here. + monkeypatch.setattr(pb, "_upstream_body_matches_assumption", lambda cls: True) + + assert pb.apply_pypowsybl_integer_value_patch() is True + assert getattr(_StubBackend, pb._PATCH_FLAG, False) is True + + inst = _StubBackend() + inst.update_integer_value(object(), np.array([0, 3, 0], dtype=np.int32), + np.array([True, True, True])) + assert calls[-1].tolist() == [-1, 3, -1] + + # Second apply is a no-op (no double-wrap): still one wrapper. + assert pb.apply_pypowsybl_integer_value_patch() is True + inst.update_integer_value(object(), np.array([0], dtype=np.int32), + np.array([True])) + assert calls[-1].tolist() == [-1] + + +# NOTE: the make_patched_pypowsybl_backend() path depends on the deprecated +# pypowsybl2grid package (removed as a dependency in v0.3.0 — its numpy==1.26.4 +# pin is unsatisfiable against numpy>=2.0.0), so its test was removed here. The +# apply_pypowsybl_integer_value_patch() tests above still cover the generic +# pypowsybl.grid2op.Backend integer-value fix, which remains in force. + + +# --- pypowsybl-installed tier (skips where absent) --------------------------- + +def test_real_pypowsybl_backend_is_patched(): + pp = pytest.importorskip("pypowsybl") + cls = pp.grid2op.Backend + assert pb.apply_pypowsybl_integer_value_patch() is True + assert getattr(cls, pb._PATCH_FLAG, False) is True + # Idempotent second call. + assert pb.apply_pypowsybl_integer_value_patch() is True + # The version guard helper returns a bool for the installed body. + assert isinstance(pb._upstream_body_matches_assumption(cls), bool) diff --git a/tests/test_simulation_optimizations.py b/tests/test_simulation_optimizations.py index 387f8142..96765eee 100644 --- a/tests/test_simulation_optimizations.py +++ b/tests/test_simulation_optimizations.py @@ -845,6 +845,8 @@ def test_empty_overload_ids(self): @pytest.mark.skipif(SKIP_RULES_TESTS, reason=RULES_SKIP_REASON) def test_empty_paths_in_validator(self): """Test validator with empty paths.""" + from expert_op4grid_recommender.action_evaluation.rules import ActionRuleValidator + from expert_op4grid_recommender.action_evaluation.classifier import ActionClassifier obs = MockObservationForSimulation( name_line=["L1", "L2"], @@ -871,6 +873,8 @@ def test_empty_paths_in_validator(self): @pytest.mark.skipif(SKIP_RULES_TESTS, reason=RULES_SKIP_REASON) def test_large_number_of_paths(self): """Test validator with large number of paths for performance.""" + from expert_op4grid_recommender.action_evaluation.rules import ActionRuleValidator + from expert_op4grid_recommender.action_evaluation.classifier import ActionClassifier # Create large lists n_lines = 1000