Config single source of truth (R3) + unified simulation seam (R4) — v0.2.8#114
Merged
marota merged 5 commits intoJul 5, 2026
Merged
Conversation
Implements review revision R3 from docs/reviews/2026-07_full_code_review.md. - Derived paths (CASE_NAME / ENV_FOLDER / ENV_PATH / ACTION_SPACE_FOLDER / ACTION_FILE_PATH / SAVE_FOLDER_VISUALIZATION) become @computed_field on Settings, so they stay in lock-step with the primary fields — overriding ENV_NAME now recomputes ENV_PATH / ACTION_FILE_PATH (fixes the A3 staleness bug). model_dump() includes them, so they are still promoted to module attrs. - Add get_settings() / override_settings() / reset_settings() accessors: override_settings runs full pydantic validation, recomputes the derived paths, and re-promotes to the module namespace. pipeline.py routes its ENV_NAME override through it instead of raw module mutation. - Collapse the 29 defensive getattr(config, 'X', default) sites to direct attribute access — every key is now a guaranteed Settings field. - Tests: delete the hand-forked tests/config_test.py and the sys.modules swap; conftest applies the test deltas through config.override_settings(...), so pydantic validation actually runs in CI (M2). Rewrite the mechanism-coupled tests and add unit tests for computed fields + accessors. Module-attribute read/write semantics are preserved (Co-Study4Grid mutates config.ENV_PATH etc. directly), so the integration contract is unchanged. Full suite parity: same 9 pre-existing lightsim2grid-drift failures, +4 net new config tests passing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014npyG1CZHF5LxRL8sa39op Signed-off-by: Claude <noreply@anthropic.com>
…ontext (R4)
Implements review revision R4 from docs/reviews/2026-07_full_code_review.md.
- Delete the ~85%-duplicated utils/simulation_pypowsybl.py; utils/simulation.py
is now the single backend-agnostic module. The two real backend differences
are explicit parameters instead of a forked file:
* simulate_kwargs — pypowsybl's keep_variant / fast_mode (grid2op: none)
* reapply_contingency — grid2op branches a candidate from the healthy
N-state and re-applies the contingency; pypowsybl
branches from the contingency-applied kept variant
and applies only the candidate.
check_rho_reduction_with_baseline now takes the branch observation and the
contract flag explicitly, structurally preventing the C-diag class of bug
(opposite first-argument contracts hidden behind identical signatures). The
defaults reproduce the grid2op contract byte-for-byte, so callers/tests that
import these functions positionally are unaffected.
- Add BaselineContext (act_defaut, baseline_rho, obs_baseline, branch_obs,
release()); _get_simulation_baseline builds it once per run and
_release_simulation_baseline frees its retained variant at the end of
discover_and_prioritize (C4). It iterates as (act_defaut, baseline_rho,
branch_obs) so the discovery call sites are unchanged.
- backends.py wires the pypowsybl simulate_kwargs / reapply_contingency;
main.py + environment_pypowsybl.py switch to the unified module.
- NetworkManager gains a kept-variant registry + LRU backstop
(register_kept_variant / sweep_kept_variants, max_kept_variants=256) so a
long-running service that reuses one NetworkManager across analyses is bounded
(C4). observation.simulate(keep_variant=True) registers each retained variant.
Behaviour-preserving: full suite at the same 9 pre-existing lightsim2grid-drift
failures; +11 new R4 unit tests; a real pypowsybl end-to-end analysis
(CHECK_ACTION_SIMULATION=True) produces the expected overloads / prioritized
actions / action_scores.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014npyG1CZHF5LxRL8sa39op
Signed-off-by: Claude <noreply@anthropic.com>
…CHANGELOG - docs/reviews/2026-07_full_code_review.md: mark R3 (config single source of truth) and R4 (unified simulation seam) as DONE (0.2.8) with what landed and what remains; mark R1/R2 as shipped (0.2.7); annotate the A3 / M2 / A8 / C4 / C7 findings and §7 quick-wins they resolve. - CHANGELOG.md: add the 0.2.8 entry; bump pyproject.toml + __init__ to 0.2.8. - CLAUDE.md: config accessors + computed derived paths, unified utils/simulation.py (+ BaselineContext), the new conftest override mechanism, the new R4 test file, and the 0.2.8 highlights. - Fix an unused-import lint in the new config test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014npyG1CZHF5LxRL8sa39op Signed-off-by: Claude <noreply@anthropic.com>
Docs: - New docs/architecture/configuration.md (R3): Settings single source of truth, computed derived paths, get_settings/override_settings/reset_settings, the module-attribute compatibility layer, and the fork-free test override. - docs/architecture/simulation-pipeline.md (R4): unified backend seam + BaselineContext + reapply_contingency, and the kept-variant registry / LRU backstop (new §2.1); refreshed the stale simulate_contingency_pypowsybl / _pypowsybl_check_with_baseline references. - docs/README.md: index both. Tests (+23): - R3: override_settings positional Settings + kwargs rejection, all-derived-path recompute, computed fields in model_dump, EXPERT_OP4GRID_* env-var parsing, LINES_DEFAUT validator (bare name + JSON), raw-mutation back-compat escape hatch. - R4: check_rho_reduction per-backend contract (grid2op re-applies from N-state / pypowsybl branches from the kept baseline), baseline-failure short-circuit, simulate_kwargs forwarding, _release_simulation_baseline lifecycle, and a real-network simulate(keep_variant=True) → registry → BaselineContext.release integration test. - New test_backends_simulation_wiring.py: each SimulationBackend forwards the right simulate_kwargs / reapply_contingency to utils.simulation. Full suite: 1759 passed / 9 pre-existing lightsim2grid-drift failures; ruff clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014npyG1CZHF5LxRL8sa39op Signed-off-by: Claude <noreply@anthropic.com>
Resolves the version-metadata conflicts between the upstream 0.2.7.post1 reassessment fix and this branch's R3/R4 work (v0.2.8): - pyproject.toml / __init__.py: version -> 0.2.8 (supersedes 0.2.7.post1) - CHANGELOG.md: keep both the 0.2.8 and 0.2.7.post1 sections - CLAUDE.md: keep 0.2.8 as current version; fold the 0.2.7.post1 note into the highlights chronology The upstream reassessment change (MIN_PARALLEL_REASSESS_WORKERS gate) merges cleanly and is preserved. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014npyG1CZHF5LxRL8sa39op Signed-off-by: Claude <noreply@anthropic.com>
marota
force-pushed
the
claude/config-simulation-refactor-182zzz
branch
from
685ccda to
af62b33
Compare
marota
pushed a commit
to marota/Expert_op4grid_recommender
that referenced
this pull request
Jul 6, 2026
…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
2 participants
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.
Implements deep revisions R3 and R4 from
docs/reviews/2026-07_full_code_review.md, released as v0.2.8. Both are behaviour-preserving (grid2op prioritized-action output byte-identical; a real pypowsybl end-to-end run verified). Builds on the R1/R2 typed-pipeline spine already shipped in v0.2.7.R3 — One config, one source of truth
Makes the pydantic
Settingsauthoritative, without locking the module down.@computed_fields (CASE_NAME,ENV_FOLDER,ENV_PATH,ACTION_SPACE_FOLDER,ACTION_FILE_PATH,SAVE_FOLDER_VISUALIZATION) — overridingENV_NAME/FILE_ACTION_SPACE_DESCrecomputes them, fixing the A3 staleness bug.model_dump()includes them, so they are still promoted to module attributes for the manyconfig.Xreaders.get_settings()/override_settings()/reset_settings().override_settingsre-validates through pydantic, recomputes the derived paths and re-promotes to the module namespace;pipeline.pyroutes itsENV_NAMEoverride through it instead of raw module mutation.getattr(config, 'X', default)sites to direct attribute access — every key is now a guaranteedSettingsfield.tests/config_test.pyand thesys.modulesswap (M2).conftest.pyapplies the test deltas viaconfig.override_settings(**TEST_CONFIG_DELTAS), soSettingsvalidation now runs in CI.config.X = ywrites + arbitrary extra attributes) — the Co-Study4Grid backend drives the recommender by mutatingconfig.*directly, so the integration contract is unchanged.R4 — Unify the per-backend simulation seam
utils/simulation_pypowsybl.py;utils/simulation.pyis a single backend-agnostic module. The two real backend differences are now explicit parameters instead of a forked file:simulate_kwargs— pypowsybl'skeep_variant/fast_mode(grid2op passes none);reapply_contingency— grid2op branches a candidate from the healthy N-state and re-applies the contingency; pypowsybl branches from the contingency-applied kept variant and applies only the candidate.check_rho_reduction_with_baselinenow takes the branch observation and the contract flag explicitly, so the opposite-first-argument-contract trap that bred the C-diag bug is gone by construction (the three injection mixins route through the same explicit seam).BaselineContext(act_defaut/baseline_rho/obs_baseline/branch_obs/release()), built once per run by_get_simulation_baselineand freed by_release_simulation_baselineat the end ofdiscover_and_prioritize(C4). It iterates as(act_defaut, baseline_rho, branch_obs)so the discovery call sites are unchanged.NetworkManagerkept-variant registry + LRU backstop (register_kept_variant/sweep_kept_variants,max_kept_variants=256) that never evicts the base or the working variant — bounds a long-running service that reuses oneNetworkManageracross analyses (cross-run half of C4).Docs & tests
docs/architecture/configuration.md;docs/architecture/simulation-pipeline.mdupdated for R4 (unified seam +BaselineContext, new §2.1 variant registry/LRU backstop). Review doc + CHANGELOG + CLAUDE.md updated; version bumped to 0.2.8.check_rho_reductioncontract,simulate_kwargsforwarding, baseline release lifecycle, backend wiring, and a real-networksimulate(keep_variant=True)→ registry →release()integration test).Verification
test_expert_op4grid_analyzercases are a pre-existinglightsim2gridversion drift (gen_slack_idloader kwarg removed upstream), unrelated to this PR and reproducible on the base commit.ruff check .clean.🤖 Generated with Claude Code
https://claude.ai/code/session_014npyG1CZHF5LxRL8sa39op