diff --git a/CHANGELOG.md b/CHANGELOG.md index 788d4e53..ae19b6d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,23 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.2.7.post1] - 2026-07-03 + +### Fixed + +- **Reassessment: stay serial on low-core hosts** (`utils/reassessment.py`). The + parallel per-action reassessment (0.2.6) clones the whole network per worker + (`save`/`load_from_binary_buffer` + `SimulationEnvironment` + observation + build) — a fixed overhead the serial path never pays, only amortized above + ~3-4 usable cores. On a **2-vCPU host** (e.g. a small HuggingFace Space) the + two clones + GIL contention on the Python-side observation build made the + "parallel" path **slower than serial** (observed ~42 s vs ~9 s on a many-core + Mac for the same 15-action pan-European case). Parallel reassessment is now + gated behind `min(10, cores, n_actions) >= 4` (env-tunable via + `EXPERT_OP4GRID_MIN_PARALLEL_REASSESS_WORKERS`; floored at 2). 2-vCPU + deployments fall back to the faster serial path; ≥4-core hosts keep the + speed-up. Behaviour and output are otherwise unchanged. + ## [0.2.7] - 2026-07-03 ### Changed diff --git a/CLAUDE.md b/CLAUDE.md index e09439b3..593e94ee 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -426,7 +426,12 @@ pytest tests/test_ActionClassifier.py::test_specific # Single test ## Current Development Status -**Current version**: `0.2.7` (see `CHANGELOG.md` for full history) +**Current version**: `0.2.7.post1` (see `CHANGELOG.md` for full history) + +> **v0.2.7.post1**: reassessment stays serial on low-core hosts — the parallel +> path's per-worker network clone is only amortized above ~4 cores, so a 2-vCPU +> Space now uses the faster serial path (gate: +> `EXPERT_OP4GRID_MIN_PARALLEL_REASSESS_WORKERS`, default 4). > **v0.2.7 highlights** (deep revisions R1 + R2 from the 2026-07 review): typed pipeline > spine — `AnalysisContext` / `AnalysisResult` dataclasses replace the ~41-key context dict diff --git a/expert_op4grid_recommender/__init__.py b/expert_op4grid_recommender/__init__.py index b933372f..a129a62d 100644 --- a/expert_op4grid_recommender/__init__.py +++ b/expert_op4grid_recommender/__init__.py @@ -15,7 +15,7 @@ import logging -__version__ = "0.2.7" +__version__ = "0.2.7.post1" _logger = logging.getLogger(__name__) diff --git a/expert_op4grid_recommender/utils/reassessment.py b/expert_op4grid_recommender/utils/reassessment.py index 12ff564d..3c533661 100644 --- a/expert_op4grid_recommender/utils/reassessment.py +++ b/expert_op4grid_recommender/utils/reassessment.py @@ -34,6 +34,47 @@ #: Hard cap on reassessment worker threads (see ``_reassessment_worker_count``). _MAX_REASSESSMENT_WORKERS = 10 +#: Default minimum worker count below which the parallel reassessment is NOT +#: worth it. The parallel path clones the whole network per worker +#: (``save``/``load_from_binary_buffer`` + ``SimulationEnvironment`` + obs +#: build) — a fixed overhead the serial path never pays. That overhead is only +#: amortized above ~3-4 usable cores; v0.2.6 measured **+1.43x on a 4-core +#: host**, but on a **2-vCPU** host (e.g. a small HuggingFace Space) the 2 +#: clones + GIL contention on the Python-side observation build make it a net +#: *loss* vs. serial. So we stay serial below the threshold. +_DEFAULT_MIN_PARALLEL_WORKERS = 4 + +#: Environment override for operators who know their hardware. +_MIN_PARALLEL_WORKERS_ENV = "EXPERT_OP4GRID_MIN_PARALLEL_REASSESS_WORKERS" + + +def _min_parallel_workers() -> int: + """Minimum worker count to enable parallel reassessment (env-tunable). + + Floored at 2 (a single worker is strictly worse than serial: it pays the + clone overhead for zero concurrency). Set the env var high (e.g. 99) to + force serial on any host, or to 2 to restore the pre-0.2.7.post1 aggressive + behaviour. + """ + raw = os.environ.get(_MIN_PARALLEL_WORKERS_ENV) + if raw is None: + return _DEFAULT_MIN_PARALLEL_WORKERS + try: + return max(2, int(raw)) + except ValueError: + return _DEFAULT_MIN_PARALLEL_WORKERS + + +def _should_parallelize_reassessment(is_pypowsybl: bool, workers: int, + n_actions: int) -> bool: + """Whether to run the reassessment in parallel. + + Parallel only pays off above a core threshold — below it the per-worker + network-clone overhead makes it slower than the serial path (see + :data:`_DEFAULT_MIN_PARALLEL_WORKERS`). + """ + return bool(is_pypowsybl) and n_actions >= 2 and workers >= _min_parallel_workers() + def _reassessment_worker_count(n_actions: int) -> Tuple[int, int]: """Return ``(cores_available, workers)`` for the reassessment pool. @@ -265,7 +306,7 @@ def _build_detail(action_id, action, obs_simu_action, info_action): sim_out: Dict[str, tuple] = {} used_workers = 1 - if is_pypowsybl and workers >= 2 and n_actions >= 2: + if _should_parallelize_reassessment(is_pypowsybl, workers, n_actions): try: main_nm = obs_simu_defaut._network_manager # Capture the N-1 baseline (contingency + maintenance) variant so diff --git a/pyproject.toml b/pyproject.toml index c766ee6f..58cdd80e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "expert_op4grid_recommender" -version = "0.2.7" +version = "0.2.7.post1" authors = [ { name="RTE", email="rte@rte-france.com" }, ] diff --git a/tests/test_reassessment_parallel_gate.py b/tests/test_reassessment_parallel_gate.py new file mode 100644 index 00000000..c7a81fe0 --- /dev/null +++ b/tests/test_reassessment_parallel_gate.py @@ -0,0 +1,88 @@ +# 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 reassessment parallel-vs-serial gate (0.2.7.post1). + +The parallel per-action reassessment clones the whole network per worker, a +fixed overhead only amortized above ~3-4 cores. Below the threshold the serial +path is faster (measured: ~42 s vs ~9 s for the same 15-action pan-European +case on a 2-vCPU Space vs a many-core Mac). These tests pin the gate so a +2-vCPU host stays serial while ≥4-core hosts keep the speed-up, plus the env +override. +""" +from __future__ import annotations + +import pytest + +from expert_op4grid_recommender.utils.reassessment import ( + _DEFAULT_MIN_PARALLEL_WORKERS, + _MIN_PARALLEL_WORKERS_ENV, + _min_parallel_workers, + _should_parallelize_reassessment, +) + + +class TestMinParallelWorkers: + def test_default_is_four(self, monkeypatch): + monkeypatch.delenv(_MIN_PARALLEL_WORKERS_ENV, raising=False) + assert _min_parallel_workers() == _DEFAULT_MIN_PARALLEL_WORKERS == 4 + + def test_env_override(self, monkeypatch): + monkeypatch.setenv(_MIN_PARALLEL_WORKERS_ENV, "2") + assert _min_parallel_workers() == 2 + monkeypatch.setenv(_MIN_PARALLEL_WORKERS_ENV, "99") + assert _min_parallel_workers() == 99 + + def test_env_floored_at_two(self, monkeypatch): + # A single worker is strictly worse than serial → never below 2. + monkeypatch.setenv(_MIN_PARALLEL_WORKERS_ENV, "1") + assert _min_parallel_workers() == 2 + monkeypatch.setenv(_MIN_PARALLEL_WORKERS_ENV, "0") + assert _min_parallel_workers() == 2 + + def test_invalid_env_falls_back_to_default(self, monkeypatch): + monkeypatch.setenv(_MIN_PARALLEL_WORKERS_ENV, "not-a-number") + assert _min_parallel_workers() == _DEFAULT_MIN_PARALLEL_WORKERS + + +class TestShouldParallelize: + def test_two_cores_stays_serial_by_default(self, monkeypatch): + # HuggingFace 2-vCPU Space: workers=2 → serial (the fix). + monkeypatch.delenv(_MIN_PARALLEL_WORKERS_ENV, raising=False) + assert _should_parallelize_reassessment(True, workers=2, n_actions=15) is False + + def test_four_cores_parallelizes(self, monkeypatch): + # The v0.2.6-measured positive case (+1.43x on a 4-core host). + monkeypatch.delenv(_MIN_PARALLEL_WORKERS_ENV, raising=False) + assert _should_parallelize_reassessment(True, workers=4, n_actions=15) is True + + def test_many_cores_parallelizes(self, monkeypatch): + monkeypatch.delenv(_MIN_PARALLEL_WORKERS_ENV, raising=False) + assert _should_parallelize_reassessment(True, workers=9, n_actions=15) is True + + def test_grid2op_never_parallelizes(self, monkeypatch): + monkeypatch.delenv(_MIN_PARALLEL_WORKERS_ENV, raising=False) + assert _should_parallelize_reassessment(False, workers=16, n_actions=15) is False + + def test_single_action_stays_serial(self, monkeypatch): + monkeypatch.delenv(_MIN_PARALLEL_WORKERS_ENV, raising=False) + assert _should_parallelize_reassessment(True, workers=8, n_actions=1) is False + + def test_env_can_force_serial_on_any_host(self, monkeypatch): + monkeypatch.setenv(_MIN_PARALLEL_WORKERS_ENV, "99") + assert _should_parallelize_reassessment(True, workers=16, n_actions=64) is False + + def test_env_can_restore_aggressive_behaviour(self, monkeypatch): + # Set to 2 → the pre-0.2.7.post1 behaviour (parallel from 2 workers). + monkeypatch.setenv(_MIN_PARALLEL_WORKERS_ENV, "2") + assert _should_parallelize_reassessment(True, workers=2, n_actions=15) is True + + +@pytest.mark.parametrize("workers,expected", [(1, False), (2, False), (3, False), + (4, True), (8, True)]) +def test_default_threshold_boundary(monkeypatch, workers, expected): + monkeypatch.delenv(_MIN_PARALLEL_WORKERS_ENV, raising=False) + assert _should_parallelize_reassessment(True, workers=workers, n_actions=15) is expected