Container-aware reassessment parallelism + R5/A5/C7 discovery refactor#115
Merged
marota merged 13 commits intoJul 6, 2026
Merged
Conversation
…ription) The per-action reassessment sized its worker pool from os.cpu_count(), which reports the HOST core count (16) inside a CPU-limited container. On a 2-vCPU cloud box it spun ~10 worker threads — each cloning a full pypowsybl network — that over-subscribed the CPU and ran slower than serial (a reported 47s reassessment). - _effective_cpu_count(): min of os.cpu_count(), scheduler affinity, 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 config REASSESSMENT_PARALLEL (None=auto | True | False) and REASSESSMENT_MIN_PARALLEL_CORES (default 4); both env-overridable via EXPERT_OP4GRID_*. On <=3 effective cores -> serial. - tests/test_reassessment_parallelism.py covers detection + gating. Behaviour-preserving otherwise; a 4-core --compare shows parallel is no faster than serial there anyway (the per-worker clone tax cancels the concurrency). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XTEMAv3usezBpo2bm2L8qu Signed-off-by: Antoine Marot <amarot91@gmail.com>
…or (R5/A5) Restructure action discovery around data instead of ~40 hand-repeated instance attributes (review A5). New discovery/_results.py holds one typed FamilyResult per family in self.results[key], described by a declarative FAMILY_SPECS registry that also drives back-compat @Property bridges — so every existing family-mixin write and test read (identified_reconnections, scores_splits_dict, scores_pst_actions, …) transparently targets the unified store with zero mixin churn. - __init__: the 39-attr quintuplet block collapses to new_results(); the 5 PST holders are now always present, so the orchestrator's getattr special-casing is gone. - action_scores assembly: the 8x hand-written 100-line literal becomes one loop over ACTION_SCORES_ORDER (byte-identical keys/order/rounding). - prioritization: the 18 hand-written add_prioritized_actions calls become two data-driven passes over MIN_PHASE_ORDER / FILL_PHASE_ORDER (one entry per family). This removes the renewable_curtailment call that had slipped in TWICE per phase — a real latent double-add (add_prioritized_actions resets its per-type counter per call, so the dup could admit up to 2x the rc floor). A no-duplicate-family invariant test locks the fix in. - PST temporal coupling (A5): replace the shared, deletable _disco_bounds / _disco_capacity_map pair (PST lazily created it, line disconnection *deleted* it at entry+exit — so PST scoring silently depended on call order across three files) with a memoised _get_disconnection_bounds() returning a frozen DisconnectionBounds. Any family may call it in any order; value is identical (pure over immutable per-run state). Behaviour-preserving: full mock-based discovery suite green (PST scores, node-splitting, min-count enforcement, action_scores shape). New coverage in tests/test_discovery_results_model.py. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XTEMAv3usezBpo2bm2L8qu Signed-off-by: Antoine Marot <amarot91@gmail.com>
…ule bypass R5: introduce a typed action-type vocabulary (action_evaluation/action_types.py) whose values are byte-identical to the historical strings (ActionType.OPEN_LINE.value == "open_line"), with category predicates (involves_line / involves_coupling / is_open / …) that give the scattered `"x" in action_type` substring checks a single home. The classifier's inline description if/elif cascade becomes one declarative, ordered classify_by_description rule table (behaviour-preserving — full classifier suite green). C7 fix (real bug): a grid2op-format coupling carries its substation only in content['set_bus']['substations_id'] (no top-level VoltageLevelId), so verify_action localized it to "unknown" and NONE of the five expert rules could fire — a silent, backend-specific rule bypass (node merges/splits in grid2op format were never filtered, while the byte-identical pypowsybl action was). New _resolve_coupling_subs() resolves the coupling substation(s) + pre-action topology from either source, and categorize_actions builds subs_topology the same way so the "No node splitting on dispatch path" guard sees the real topology. The pypowsybl / REPAS (VoltageLevelId) path is unchanged. Regression coverage: tests/test_action_types_enum.py + two C7 tests in tests/test_ActionRuleValidator.py. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XTEMAv3usezBpo2bm2L8qu Signed-off-by: Antoine Marot <amarot91@gmail.com>
…on families (R5) Load shedding, renewable curtailment and redispatch each opened with the same ~15-line overload preamble (warm caches, read the family margin / min-MW knobs, compute max_overload_flow + P_overload_excess) and the same saturating influence-factor formula — duplicated 3x and already drifting. New discovery/_injection_base.py::InjectionDiscoveryBase factors out exactly those two byte-identical shared pieces (_injection_overload_context returning a frozen InjectionOverloadContext, _injection_influence_factor); each family sets MARGIN_KEY / MIN_MW_KEY. The three mixins now inherit it. Deliberately NOT unified (the divergence is load-bearing, documented in the module): which node-flow cache is built (blue-only for load shedding vs. blue+dispatch for the generator families — coupled to how many flow components each family's _influence_of reads), the candidate loop, the per-candidate simulation-check mode, the mw_required arrangement (load shedding keeps its P_shedding clamp to avoid ULP drift), and the family-specific params schema. Behaviour-preserving: the full mock-based injection-scoring suite (overload excess, influence factor, score ranges, params structure) stays green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XTEMAv3usezBpo2bm2L8qu Signed-off-by: Antoine Marot <amarot91@gmail.com>
…le tests + C6 verify (R6) A5 (last item): the family mixins reached into `self` for shared state and helpers with nothing declaring that contract. New discovery/_protocols.py DiscovererProtocol writes it down in one @runtime_checkable place (the observations, the FamilyResult store, the cache accessors + memoised baselines a mixin may assume), with a conformance test so "the base lost a helper a mixin needs" fails as a test rather than a runtime AttributeError deep in discovery. R6: give the two previously-untested data modules (utils/load_training_data.py, utils/load_evaluation_data.py — review M3 blind spot, and exactly where the C6 bugs hid) their first tests (tests/test_data_modules.py): import smoke, the pure load_interesting_lines CSV reader, and regression guards for the C6 fixes (filter_out_non_reproductible_observation now requires line_we_disconnect as a parameter; the no-chronic path raises ValueError, not a bare raise). Verified all four C6 bugs are already fixed in the current tree. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XTEMAv3usezBpo2bm2L8qu Signed-off-by: Antoine Marot <amarot91@gmail.com>
…status Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XTEMAv3usezBpo2bm2L8qu Signed-off-by: Antoine Marot <amarot91@gmail.com>
…+ rc no-double-add; docs - tests/test_injection_base.py: _injection_influence_factor (pure, clamp/zero-ref), _injection_overload_context (None early-returns + happy path + capacity fallback), and _get_disconnection_bounds memoisation (computed once, frozen holder). - tests/test_discovery_results_model.py: behavioural regression proving the production MIN/FILL phase-order tables cap renewable_curtailment at its fill cap (the R5 double-add fix), complementing the structural no-dup invariant. - CLAUDE.md: document the R5 discovery data model (_results.py FamilyResult / registry / property bridges / phase tables), _injection_base.py, _protocols.py, the ActionType enum + declarative classifier, the C7 backend-agnostic coupling localization, and the new test files. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XTEMAv3usezBpo2bm2L8qu Signed-off-by: Antoine Marot <amarot91@gmail.com>
…uent-work section Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XTEMAv3usezBpo2bm2L8qu Signed-off-by: Antoine Marot <amarot91@gmail.com>
Bump version to 0.2.9 across pyproject.toml + __init__.py; promote the [Unreleased] changelog block (R5/A5 discovery-around-data, ActionType + C7 rule-bypass fix, R6-partial, container-aware reassessment) to [0.2.9]; add docs/release-notes/v0.2.9.md and refresh the CLAUDE.md highlights + docs index. 0.2.8 was published to PyPI outside this repo's history and is skipped to avoid a version collision. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XTEMAv3usezBpo2bm2L8qu Signed-off-by: Antoine Marot <amarot91@gmail.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XTEMAv3usezBpo2bm2L8qu Signed-off-by: Antoine Marot <amarot91@gmail.com>
marota
force-pushed
the
claude/co-study4grid-perf-benchmark-s1u2xr
branch
from
2960204 to
ceb88e4
Compare
… summary Fold the deep-revision done/partial/deferred status from the top narrative into §6 as inline annotations (mirroring §4.1's 'FIXED in this branch' tags), one per R1-R10, and replace the top section with a concise summary of the remaining roadmap. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XTEMAv3usezBpo2bm2L8qu Signed-off-by: Antoine Marot <amarot91@gmail.com>
… categories Expand the top status summary beyond the deep-revision roadmap to cover open items in every review category — correctness/security residuals (§4.1/C7/W3), architecture (§4.2), performance (§4.3), maintainability/tests/packaging (§4.4) and the §7 quick wins — cross-referencing the deep revision that subsumes each. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XTEMAv3usezBpo2bm2L8qu Signed-off-by: Antoine Marot <amarot91@gmail.com>
…xt) into the 0.2.9 branch Sync the fork with upstream ainetus/main, which had advanced to 0.2.8 (deep revisions R3 + R4 via PR ainetus#114, plus 0.2.7.post1) while this branch built 0.2.9 (R5/A5/C7 + container-aware reassessment) on the older 0.2.7 base. Conflict resolutions: - Version -> 0.2.9 (successor to 0.2.8); CHANGELOG keeps [0.2.8] R3/R4 + [0.2.7.post1] below [0.2.9]; dropped the now-moot 'skip 0.2.8' note. - reassessment.py: unified the two 'stay serial on small hosts' approaches into one coherent gate — container-aware _effective_cpu_count() (ours) feeds the worker pool; _should_parallelize_reassessment() (theirs) is the single gate, now honoring the REASSESSMENT_PARALLEL force knob and a threshold unified with the EXPERT_OP4GRID_MIN_PARALLEL_REASSESS_WORKERS env var. Both test suites green. - discovery injection files + orchestrator: kept R5 (InjectionDiscoveryBase _ctx preamble + data-driven FILL_PHASE_ORDER loop that removes the rc double-add); R4's BaselineContext iterates as the old 3-tuple so the call sites are unchanged. _base.py carries both R4 (BaselineContext/release) and R5 (self.results/_get_disconnection_bounds). - test_discovery_package_structure: method count reconciled to 53 (R4 release helper + R5 bounds/injection helpers). - review doc: R3 and R4 now correctly recorded as DONE (0.2.8); top 'what's left' and §6 reconciled. CLAUDE.md/docs: merged R3/R4 + R5 narratives; added the marota<->ainetus fork-sync instruction. Validated: 416 mock tests pass; ruff clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XTEMAv3usezBpo2bm2L8qu Signed-off-by: Antoine Marot <amarot91@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
• Container-aware CPU detection (utils/reassessment.py): read cgroup v2 cpu.max / cgroup v1 quota+period and sched_getaffinity instead of os.cpu_count(), so a 2-vCPU container no longer spawns 10 workers. New config knobs REASSESSMENT_PARALLEL (None/auto, True, False) and REASSESSMENT_MIN_PARALLEL_CORES.
• R5 — data-driven discovery: FamilyResult/FAMILY_SPECS model + data-driven action_scores assembly and two-pass prioritization (fixes a latent renewable-curtailment double-add); ActionType enum + declarative classify_by_description.
• A5 — real modularization of the discovery split: DiscovererProtocol, shared InjectionDiscoveryBase, memoised _get_disconnection_bounds().
• C7 — backend-agnostic coupling localization in rules.py (_resolve_coupling_subs): grid2op couplings no longer localize to "unknown" and bypass every expert rule.
• Behaviour-preserving (byte-identical prioritized-action output); new tests test_reassessment_parallelism, test_discovery_results_model, test_action_types_enum, test_data_modules, test_injection_base + C7 rule tests; CLAUDE.md / CHANGELOG / review-doc updated.