Bus connectivity IS the element counts, and nothing else by BDonnot · Pull Request #188 · Grid2op/lightsim2grid

BDonnot · 2026-08-30T12:00:55Z

Rebased onto dev_1.0.1 after #185 merged, so this is 5 commits on top of it — no stack.

The problem

std::vector<bool> bus_status_ said the same thing as "does any element sit on this bus", a second time. Two statements of one fact can disagree, and this pair did:

// LSGrid::init_bus_status(), before
substations_.disconnect_all_buses();
powerlines_.reconnect_connected_buses(substations_);
shunts_.reconnect_connected_buses(substations_);
trafos_.reconnect_connected_buses(substations_);
generators_.reconnect_connected_buses(substations_);
loads_.reconnect_connected_buses(substations_);
sgens_.reconnect_connected_buses(substations_);
storages_.reconnect_connected_buses(substations_);
hvdc_lines_.reconnect_connected_buses(substations_);
// svcs_ is not on this list

SvcContainer is a OneSideContainer_PQ and inherits a perfectly good reconnect_connected_buses. It was simply left off. A bus whose only element was an SVC read as disconnected, got no solver id, and silently dropped out of the system it should have been in. An SVC injects reactive power; its bus belongs there.

That is what a hand-written list does. It was also O(all elements) on every powerflow that rebuilt, it was serialized (so a crafted file could make it contradict the elements it claims to describe), and it could be set by hand — deactivate_bus — to something the elements contradicted.

The fix

SubstationContainer keeps, per bus, how many elements hold it alive. A bus is in the solved system iff that count is non-zero, so the two transitions that matter (0 -> 1, 1 -> 0) are exactly the ones that change the dimension of the system and renumber every bus after them. Those, and only those, raise tell_dimension_changed.

	before	after
`is_bus_connected(b)`	`bus_status_[b]`	`nb_elements_per_bus_[b] > 0`
`nb_connected_bus()`	walk the whole vector	one integer, moved by one at the crossings
`get_bus_status()`	`const std::vector<bool>&` into a member	built from the counts, by value
`init_bus_status()`	disconnect all + walk 8 containers	nothing, in the steady state
dimension-change detection	two O(nb_bus) walks comparing against a per-family photograph	the crossing raises the flag as it happens, O(1)

Which buses an element holds is stated once, in GenericContainer::contribute_to_buses. The recount, and every mutator's -1 … mutate … +1 bracket, are built from that one predicate — so the incremental and the rebuilt answer cannot disagree about a rule. The rule is genuinely per container, which is why it is virtual:

one-sided elements hold their bus iff active;
a line / trafo is gated by status_global_ first, and only then does each side count — a line end cannot know that, so it cannot live in OneSideContainer;
an HVDC line has no global gate (one station on while the other is off is legitimate) and simply delegates to its two sides. Behaviour left exactly as it was, per review.

Only tell_dimension_changed moves here. ybus_change_sparsity_pattern / ybus_values_changed stay where the shunt / trafo / line containers decide them.

Deleted rather than kept

reconnect_connected_buses, all five containers. Dead once the counts drive the status, and it was the second statement of the rule — the one that drifted.
OneSideContainer's "a connected element's bus must be active" check in check_grid. An active element on bus b is an element holding b, so counts[b] > 0: a tautology. It could only ever fire before the counts were established, which says nothing about the grid.
GenericContainer::_generic_deactivate/_generic_reactivate(GlobalBusId, SubstationContainer&) — no caller left.

Deprecated no-ops

LSGrid.deactivate_bus / reactivate_bus (and SubstationContainer's disconnect_all_buses / reconnect_bus / disconnect_bus / reset_bus_status) have nothing left to set. Kept so existing code keeps importing — the pandapower and powermodels loaders and LightSimBackend all call them.

This changes no powerflow result, because they were already all but inert: whatever they wrote, the next powerflow rebuilt the status from the elements and threw it away. A bus with elements on it came straight back; a bus without them was already out. What actually took an out-of-service bus out of the system was the loaders disconnecting its elements, and that still works.

Recovery, so drift cannot outlive an invalidation

Incremental bookkeeping rots silently — one missed decrement and the count is wrong for the rest of the grid's life. So the counts are recounted from the elements whenever there is nothing to protect: a freshly built grid, set_state / load_binary, and any init_* that replaces a whole element container (which no +1 / -1 can see).

⚠️ Binary format 4 -> 6

Nothing about bus connectivity is serialized any more: LSGrid's state loses the AC family's bus-connectivity photograph (5), SubstationContainer's loses bus_status_ (6). A restored grid counts its buses from its elements, whose own status is in the file — so the connectivity round-trips exactly and a file cannot state anything else.

test_check_grid's "rejects a bus_status shorter than the bus count" goes with the field: it poisoned a length a file can no longer state. Replaced from the other side by "a restored grid counts its own buses", which changes the topology, round-trips, and checks the restored grid agrees bus for bus.

Tests

src/tests/test_bus_element_count.cpp is new. All 41 mutators that can change bus membership, each on a cold grid and on one that has already solved, against the strong assertion: after ANY mutation, the incremental counts equal what a recount from the elements produces. Plus the no-op cases (moving an element to the bus it is already on; deactivating an already-deactivated element) — grid2op sends a topology vector every step, most of which asks for the bus an element is already on, and a transient 1 -> 0 -> 1 would read as "the dimension changed" and cost a full rebuild.

connected_bus_count_is_exact() states the one remaining invariant and is checked, not asserted in a comment: init_bus_status() asserts it, and the sweep checks it after every mutator, before the recount that would re-establish it and hide drift.

A guard on the list itself: CHECK(all_bus_mutations().size() == 41u), so a new mutating method that is not added there stops the build's conscience rather than silently escaping coverage.

What it is worth

Callgrind, one load moved between two buses per solve (so init_bus_status() actually runs), 21 solves each:

grid (sub × busbar)	nb_bus	before	after	Δ	per bus
1000 × 1	1 000	16 378 616	16 365 863	−0.08%	−12.8
1000 × 12	12 000	16 990 176	16 832 723	−0.93%	−13.1
5000 × 1	5 000	121 603 588	121 539 360	−0.05%	−12.8
5000 × 12	60 000	124 767 349	123 979 613	−0.63%	−13.1

Divided by nb_bus: 12.8, 13.1, 12.8, 13.1 — one flat constant, the same at every size and every element count. The cost removed was proportional to the number of buses, not to the work the solve does, so it is the sparsely-filled grids that gain: 5000 substations at 12 busbars each is a 60 000-entry vector rebuilt to solve a 5 000-bus system.

In wall clock it does not show at any of the four sizes; the solve dominates and run-to-run spread is a few percent. The case for this change is the shape, and the SVC bug it makes unspeakable. The instructions are a bonus, not the argument.

Verification

215 test cases / 7327 assertions pass under Release, C++14 (LS2G_CXX_STANDARD=14), Debug (assertions live), ASan+UBSan, and valgrind (0 errors, no leaks).

The python layer could not be exercised here — no pandapower / pypowsybl / grid2op on this machine. CI covers it.

⚠️ One thing this PR cannot fix

lightsim2grid/tests/binary_format_fixture/case14_sandbox_format4.lsb is a format-4 file and was not regenerated at the 4 -> 5 bump, so TestBinaryLayoutUnchanged fails independently of this work. Regenerating needs grid2op:

python -m lightsim2grid.tests.test_binary_serialization regen

then rename the file and FIXTURE_PATH to ..._format6.lsb.

🤖 Generated with Claude Code

https://claude.ai/code/session_01UtvkqXMvgCig75gruZCs7u

Generated by Claude Code

SubstationContainer now keeps, per bus, how many elements hold it alive. Nothing consumes it yet: this commit is the bookkeeping and its tests, so the switch-over that follows is a small diff on a mechanism already proven. Why a count and not a snapshot ------------------------------ Today connectivity is a std::vector<bool> photograph, rebuilt by init_bus_status() walking every element of eight containers, copied per powerflow by _mark_cache_valid, and compared bus-by-bus twice by _flag_dimension_change. A count answers the only question that matters -- has a bus entered or left the solved system -- in O(1), because that is exactly a 0 <-> 1 crossing. One statement of "which buses does this element hold" ----------------------------------------------------- The rule is genuinely different per container, and encoding it twice is how this kind of bookkeeping rots: - one-sided elements hold their bus iff active; - a line or transformer is gated by status_global_ FIRST, then each side counts on its own status -- a line END cannot know that, so it cannot own the decision; - an HVDC line has no such gate: each converter station stands alone (kept exactly as it is today, per review). So there is one virtual, GenericContainer::contribute_to_buses, and reconnect_connected_buses, the from-scratch recount and every mutator are all built from it. Nothing restates the rule. Mutators do not do arithmetic ----------------------------- _apply_and_track_buses, written once, brackets any mutation: take this element's current contribution away, mutate, put the new one back. Whatever the container's rule and whatever the mutation did, the counts land right -- no per-mutator +1/-1 to get wrong. 42 call sites, all the same three lines. A nested call must not count for itself: for a line, the sides' own contribution is UNGATED, so a side counting during a branch-level mutation would decrement a bus the gate says the branch never held. Hence deactivate_no_bus_tracking / reactivate_no_bus_tracking / change_bus_no_bus_tracking, which TwoSidesContainer calls inside its own bracket -- including from resolve_status, where synch_status_both_side_ moves the other end. Signature: deactivate / reactivate gain SubstationContainer& (change_bus already had it; change_bus_side_* went from const& to &). Only tell_dimension_changed --------------------------- A crossing raises tell_dimension_changed and nothing else. Every ybus_change_sparsity_pattern / need_recompute_ybus / need_recompute_sbus stays exactly where the containers decide it, untouched. Two things this turned up ------------------------- - OneSideContainer's public deactivate/reactivate validated el_id before dispatching, precisely because _deactivate indexes status_[el_id] unchecked. _apply_and_track_buses reads the contribution BEFORE the mutation, which moved that read in front of the check -- an out-of-range id segfaulted instead of throwing. Caught by test_lsgrid's "element setters validate the element id"; the check is now back in front, in every tracked wrapper. - svcs_ is missing from init_bus_status()'s reconnect list. SvcContainer is a OneSideContainer_PQ and inherits a working reconnect_connected_buses, it is simply never called -- so a bus whose ONLY element is an SVC does not count as connected today. The counts include it (excluding it would make SvcContainer the one container that does not track itself, the exact special case this design avoids), so they differ from bus_status_ for an SVC-alone bus. Nothing consumes the counts, so nothing changes; whoever switches the consumer over closes that gap and owes it a test. Counts start disarmed: all-zero is indistinguishable from "every bus is empty", so tracking would underflow on the first deactivate_load of a freshly built grid. recompute_bus_element_counts() arms them, and init_bus_status() runs it -- the same "recount whenever there is no cache to protect" rule that stops drift outliving an invalidation, applied to the moment before the first powerflow. They are deliberately absent from StateRes: derived state, recomputed on load. Tests: src/tests/test_bus_element_count.cpp. The only assertion worth making against incremental bookkeeping is the strong one -- after ANY mutation the counts equal a fresh recount -- so that is what all 41 mutators are checked against, each on a cold grid and on one that has already solved. Plus the no-op cases the guards exist for (a bus changed to itself, an already-deactivated element deactivated again) and a count on the sweep itself, so a new mutator cannot be added without noticing this file. Verified: 213 test cases / 7119 assertions pass under Release, C++14, Debug (assertions live, including the drift assert), ASan+UBSan, and valgrind (0 errors, no leaks). The python layer could not be exercised here (no pybind11 / numpy on this machine); no python-visible API changed. Signed-off-by: DONNOT Benjamin <benjamin.donnot@rte-france.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UtvkqXMvgCig75gruZCs7u

The counts from the previous commit now drive `tell_dimension_changed`, and the bus-connectivity photograph they replace is gone -- from the per-powerflow path, from SolverBusLayout, and from the binary format. What each powerflow used to pay, and no longer does: _mark_cache_valid cache.last_bus_status = substations_.get_bus_status() a full std::vector<bool> copy, every powerflow -> cache.built_for_nb_bus = nb_bus, one integer init_bus_status() disconnect_all_buses(), then every element of eight containers put back -- O(all elements) -> set_bus_status_from_element_counts(), O(nb_bus), touching no element _flag_dimension_change two O(nb_bus) walks, one per family, comparing the fresh status against each family's photograph -> deleted. The crossing that would have produced the difference raises the flag as it happens, in GenericContainer::_apply_and_track_buses -- earlier, exact, and O(1). `SolverBusLayout::last_bus_status` becomes `std::size_t built_for_nb_bus`, which is all `is_consistent` ever wanted from it, and doubles as the retirement marker (0 = never built / retired). Binary format ------------- Slot 6 of LSGrid::StateRes held that photograph. Nothing needs it any more, so it is removed rather than left as a dead field: the index enum is renumbered and BINARY_FORMAT_VERSION goes 4 -> 5. That also deletes an attack surface instead of defending it. test_cache_reuse's "a hand-edited bus-connectivity snapshot cannot authorize any reuse" poisoned that field -- the one piece of cache metadata a serialized grid carried -- and checked that a crafted file still could not authorize reuse. There is nothing left to poison. The section now tests the property it was protecting, which is still live because element status IS serialized: restoring a grid with a different topology into a warm one must leave it cold and answer like a grid that never cached. Behaviour change, deliberate: `svcs_` was missing from init_bus_status()'s reconnect list (SvcContainer is a OneSideContainer_PQ and inherits a working reconnect_connected_buses, it was simply never called), so a bus whose ONLY element is an SVC did not count as connected. The counts include it, so deriving the status from them closes that gap. An SVC injects reactive power; its bus belongs in the solved system. What it is worth ---------------- Callgrind, a chain grid of 1000 buses, one load moved between two buses per powerflow so init_bus_status() actually runs; slope between 40 and 240 solves: baseline (before the counts) 16 081 442 instr/pf counts maintained, unused 16 307 609 +226 166 (+1.41%) counts drive it (this) 15 805 534 -275 909 (-1.72%) The middle row is this pair's cost before the payoff: adding the counts put a SECOND full element walk inside init_bus_status(), which is why it costs about what the walk it replaces costs. Here that recount becomes one-time (guarded by bus_counts_ready()), so the steady state is O(nb_bus) -- 3.1% fewer instructions than the intermediate state, 1.7% fewer than before any of it. In wall clock none of this shows: on chains of 118 / 1000 / 5000 buses, with and without a topology change per solve, the three builds sit within each other's run-to-run spread (~4%). The solve dominates. So the case for this change is the mechanism -- one statement of connectivity, no snapshot to keep in sync, one fewer field in the binary format -- and the instructions are a bonus, not the argument. (The first version of this benchmark was wrong twice and both are worth knowing: the synthetic chain was electrically infeasible, so NR diverged, burned max_iter and rebuilt every call while silently reporting 3.3 ms for a 118-bus solve -- the harness now aborts on a diverged solve rather than timing one; and the three binaries were compiled against one set of headers while linked against three different libraries, which with this commit's member-layout change produced plausible-looking runtime errors rather than a link failure. Each variant is now built against its own headers.) Verified: 213 test cases / 7120 assertions pass under Release, C++14, Debug (assertions live), ASan+UBSan, and valgrind (0 errors, no leaks). The python layer could not be exercised here (no pybind11 / numpy on this machine). Signed-off-by: DONNOT Benjamin <benjamin.donnot@rte-france.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UtvkqXMvgCig75gruZCs7u

The previous commit made `init_bus_status()` read the per-bus element counts instead of walking every element of eight containers. That was still an O(nb_bus) rewrite of the whole status vector on every powerflow that rebuilds -- which is the wrong shape, and the wrong shape in the direction that hurts: with 5000 substations at 12 busbars each it walks 60 000 entries to solve a 5 000-bus system, and the ratio only gets worse as a grid gets more busbars per substation. It does not have to be O(anything). A bus is connected iff its element count is non-zero, so its status can change at exactly two moments: the 0 -> 1 and the 1 -> 0 crossings. Those are already detected -- `bus_gained_element` / `bus_lost_element` return them, that is how `tell_dimension_changed` is raised. So they now also flip that one entry of `bus_status_`, and the vector is simply correct at all times, with no walk anywhere. void set_bus_status_from_element_counts() O(nb_bus), every rebuild -> bus_status_[idx] = true; O(1), only when a bus crosses 0 What is left of the rebuild --------------------------- Two cases, and `bus_status_needs_refresh()` is what distinguishes them: - nothing has counted yet (a freshly built grid, or one restored by set_state / load_binary, which disarm the counts). Recount from the elements, then derive the status from it -- the same "recount whenever there is nothing to protect" rule that keeps drift from outliving an invalidation. - something wrote `bus_status_` behind the counts' back. That is exactly three methods -- `deactivate_bus`, `reactivate_bus`, `disconnect_all_buses` -- which change a bus's status without changing what holds it. They say so, and the next rebuild re-derives the whole vector, which is precisely what the old unconditional rebuild did to a hand-set status too. No behaviour changes. `init_*` disarms the counts --------------------------- Replacing a whole element container (`init_loads` and the twelve others) is not something the incremental +1 / -1 can see, so counts taken before it would go on describing elements that no longer exist. They are now disarmed there, and the next `init_bus_status()` recounts. This was already a latent hole in the previous commit; it is closed here rather than left for the assertion below to find. Keeping it honest ----------------- `bus_status_matches_counts()` states the invariant -- `bus_status_[i] == (count[i] > 0)` for every bus -- and it is checked, not asserted in a comment: - `init_bus_status()` asserts it (debug builds, which CI runs under ASan/UBSan and valgrind). Free in release. - the mutator sweep in test_bus_element_count.cpp checks it after every one of the 41 mutators, before the recount that would re-derive it and hide drift. - a new case covers the two cases above and the `init_*` one directly. The SVC gap that the previous commit noted and left open ("whoever switches the consumer over closes that gap, and owes it a test") is closed and has its test: an active SVC contributes +1 to its bus, so a bus holding only an SVC has a count of 1 and is in the solved system. The comment claiming otherwise is corrected. What it is worth ---------------- Callgrind, a topology-changing powerflow (one load moved between two buses per solve), four shapes of the same chain grid, 21 solves each: grid (sub x busbar) nb_bus base instr/pf this instr/pf delta 1000 x 1 1 000 16 378 616 16 365 863 -0.08% 1000 x 12 12 000 16 990 176 16 832 723 -0.93% 5000 x 1 5 000 121 603 588 121 539 360 -0.05% 5000 x 12 60 000 124 767 349 123 979 613 -0.63% Divided by nb_bus that is 12.8, 13.1, 12.8, 13.1 instructions saved per bus -- one flat constant, the same at every size and at every element count. That is the point: the cost removed was proportional to the number of buses, not to the work the solve actually does, so it is the sparsely-filled grids that gain, and it is exactly the case2 caveat this pair of commits was missing. In wall clock it does not show, at any of the four sizes: the solve dominates and the run-to-run spread is a few percent. The case for the change is the shape. Verified: 215 test cases / 7287 assertions pass under Release, C++14, Debug (assertions live), ASan+UBSan, and valgrind (0 errors, no leaks). The python layer could not be exercised here (no pandapower / pypowsybl on this machine). Signed-off-by: DONNOT Benjamin <benjamin.donnot@rte-france.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UtvkqXMvgCig75gruZCs7u

`std::vector<bool> bus_status_` said the same thing as the per-bus element counts, a second time. Two statements of one fact can disagree, and this pair did: the SVC gap fixed two commits ago was exactly that -- the rebuild walked a hand-written list of eight containers, SvcContainer was not on it, and a bus held only by an SVC silently dropped out of the solved system. It also had to be kept true (an O(nb_bus) rewrite per powerflow), it was serialized (so a crafted file could make it contradict the elements it claims to describe), and it could be set by hand to something the elements contradicted. It is gone. `is_bus_connected(b)` is `nb_elements_per_bus_[b] > 0`, and there is no other answer to give. is_bus_connected(b) bus_status_[b] -> counts[b] > 0 nb_connected_bus() a walk over the whole vector -> one integer, moved by one at the crossings get_bus_status() a reference to the member -> built from the counts, BY VALUE init_bus_status() rewrite nb_bus entries -> nothing, in the steady state Deprecated no-ops ----------------- `deactivate_bus` / `reactivate_bus` (and SubstationContainer's `disconnect_all_buses` / `reconnect_bus` / `disconnect_bus` / `reset_bus_status`) have nothing left to set, so they do nothing. They are kept so existing code keeps importing -- the pandapower and powermodels loaders and LightSimBackend all call them. This changes no powerflow result, because they were already all but inert: whatever they wrote, the next powerflow rebuilt the status from the elements and threw it away. A bus with elements on it came straight back; a bus without them was already out. What actually took an out-of-service bus out of the system was the loaders disconnecting its ELEMENTS, and that still works. Two checks are deleted rather than kept, because both became tautologies: - OneSideContainer's "a connected element's bus must be active" (check_grid). An active element on bus b IS an element holding b, so `counts[b] > 0`. It could only ever fire before the counts were established, which says nothing about the grid. - `reconnect_connected_buses`, in all five containers. It was already dead -- nothing has called it since init_bus_status() started reading the counts -- and it was the second statement of "which buses does this element hold", the one that drifted. `contribute_to_buses` is now the only one. Binary format 5 -> 6 -------------------- `SubstationContainer::StateRes` loses its `std::vector<bool>` slot. A restored grid counts its buses from its elements, whose own status IS serialized, so the connectivity round-trips exactly and a file cannot state anything else. test_check_grid's "rejects a bus_status shorter than the bus count" goes with the field: it poisoned a length a file can no longer state. The property it protected is covered from the other side by a new case, "a restored grid counts its own buses", which changes the topology, round-trips, and checks the restored grid agrees bus for bus. Keeping it honest ----------------- `connected_bus_count_is_exact()` states the one remaining invariant -- the maintained `nb_connected_bus_` equals a fresh count of the non-empty buses -- and it is checked, not asserted in a comment: `init_bus_status()` asserts it (debug builds, which CI runs under ASan/UBSan and valgrind), and the mutator sweep checks it after every one of the 41 mutators, before the recount that would re-establish it and hide drift. Known follow-up, not fixable here --------------------------------- lightsim2grid/tests/binary_format_fixture/case14_sandbox_format4.lsb is a format-4 file and has not been regenerated since the 4 -> 5 bump two commits ago, so TestBinaryLayoutUnchanged fails on this branch already. Regenerating it needs grid2op, which is not installed on this machine: python -m lightsim2grid.tests.test_binary_serialization regen then rename the file and FIXTURE_PATH to ..._format6.lsb. Verified: 215 test cases / 7318 assertions pass under Release, C++14, Debug (assertions live), ASan+UBSan, and valgrind (0 errors, no leaks). The python layer could not be exercised here (no pandapower / pypowsybl / grid2op on this machine). Signed-off-by: DONNOT Benjamin <benjamin.donnot@rte-france.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UtvkqXMvgCig75gruZCs7u

The three commits added to #185 before it merged (`LSGrid::reset()` clearing a cache, the split into two entry points, the changelog moving to a new [1.0.1]) overlap this branch. The rebase resolved the code; this is what it could not: - Two of the comments #185 merged with describe a "connectivity snapshot" that these commits delete. Rewritten to say what is actually there (`built_for_nb_bus`, and the element counts), in `reset()`, `set_state()`, the `build_into_cache` doc and `_mark_cache_valid`. - The changelog described two format bumps, 4 -> 5 and then 5 -> 6, inside one unreleased section. Nobody ships 5. Folded into one 4 -> 6 entry that says what the release actually does: nothing about bus connectivity is serialized. - The counts entry credited `reconnect_connected_buses` as being built from `contribute_to_buses`; a later commit in this same section deletes it. It now says so, and points at the SVC fix as the reason. No code change. 215 test cases / 7327 assertions pass under Release, C++14, Debug, ASan+UBSan and valgrind. Signed-off-by: DONNOT Benjamin <benjamin.donnot@rte-france.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UtvkqXMvgCig75gruZCs7u

Two paths in #188 change which buses an element holds without going through the counting bracket, so the system silently loses a bus and the next powerflow fails to initialise (ErrorType.NotInitError). update_topo(), the entry point grid2op drives every step, is not a thin wrapper over the individual mutators: it also calls resolve_status, which sets a branch's status_global_ -- the gate contribute_to_buses reads FIRST -- and opens the opposite side through the *_no_bus_tracking entry points. Both ran outside the bracket. Strand a line end alone on a busbar and then disconnect the line, and that bus stayed counted with nothing on it. The whole per-element update is now ONE bracket, both sides and resolve_status inside it, with the sides using the *_no_bus_tracking entry points for the reason deactivate() already does: a line end does not own its contribution. This also collapses a one-sided element's "reactivate then move" into one bracket instead of two. consider_only_main_component() had the crossings right and threw them away: disconnect_if_not_in_main_component handed every deactivation a local, throwaway DualAlgoControl. Before #188 the flag still reached the real controller, because init_bus_status() rebuilt the status and compared it against each family's photograph. Nothing compares photographs any more -- the crossing IS the notification -- so it now goes to the controller the solver reads. The two-sided containers also do their counting through the branch's rule, in one bracket around both sides and the status_global_ flip. This is what broke grid2op's automatically_disconnect=True (test_detach_if_not_main_comp). Tests: three new cases in test_bus_element_count.cpp, each verified to fail without its fix -- update_topo taking a bus out, update_topo re-asserting the bus an element is already on (the no-op grid2op sends every step, which must not report a crossing), and consider_only_main_component raising the dimension flag on the real controller. The 41-mutator sweep could not reach either path. Also brings the python tests in line with the format 4 -> 6 bump that #188 already made in C++: the substation block moved to index 6 of LSGrid's state and lost its bus_status field, and the reference fixture is regenerated as case14_sandbox_format6.lsb. Signed-off-by: DONNOT Benjamin <benjamin.donnot@rte-france.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E8DwbumaiafbKSb53BCodR

The per-bus element counts are disarmed by everything the incremental +1 / -1 cannot see: a freshly built grid, set_state / load_binary, an init_* that replaces a whole element container. An all-zero count is exactly what "never counted" looks like, so a disarmed grid answers "every bus disconnected". Only the powerflow re-established them, through init_bus_status(). But the connectivity readers are public API and a loader is entitled to be asked before anything is solved -- init_from_matpower(...) followed by get_bus_status() reported all four buses out (test_init_from_matpower's TestInitFromMatpowerNBusbarPerSub). get_bus_status() and nb_connected_bus() now establish the counts themselves, and init_bus_status() is that same step under its old name. Also brings the remaining python state-index constants in line with the format 4 -> 6 layout: LSGrid's state lost the AC family's bus-connectivity photograph, so line/shunt/trafo/hvdc/svc and the substation block all shift down by one, and the substation block itself lost its bus_status field, moving bus_vmin/bus_vmax with it. Signed-off-by: DONNOT Benjamin <benjamin.donnot@rte-france.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E8DwbumaiafbKSb53BCodR

Review nit on OneSideContainer.hpp: `sub_id` is read once from `subid_` and only ever passed to the range check and local_to_gridmodel, so it has no business being mutable. Signed-off-by: DONNOT Benjamin <benjamin.donnot@rte-france.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E8DwbumaiafbKSb53BCodR

BDonnot

Things to change I think

BDonnot · 2026-08-31T11:17:10Z

         * @return int 
         */
-        int nb_connected_bus() const {return substations_.nb_connected_bus();}
+        int nb_connected_bus() const {_ensure_bus_counts(); return substations_.nb_connected_bus();}


I am not a fan here. Except for the first call before a powerflow, we know exactly this in O(1) for all successive checks. This is weird to force an O(n_bus) each time

BDonnot · 2026-08-31T11:17:52Z

-        [[nodiscard]] const std::vector<bool> & get_bus_status() const {return substations_.get_bus_status();}
+        /// which buses are in the solved system; built from the element counts, see
+        /// SubstationContainer::get_bus_status (returns BY VALUE, no longer a reference)
+        [[nodiscard]] std::vector<bool> get_bus_status() const {_ensure_bus_counts(); return substations_.get_bus_status();}


Same comment as for nb_connected_buses, should be O(1) once init

BDonnot · 2026-08-31T11:23:41Z

    [[nodiscard]] bool layout_is_consistent(std::size_t nb_bus_grid) const noexcept {
        const Eigen::Index nb_solver = nb_bus_solver();
        if(nb_solver == 0) return false;            // never built
        if(id_me_to_solver.size() != nb_bus_grid) return false;  // built for another grid size
        if(slack_weights.size() != nb_solver) return false;
        // every bus is pv, pq, or slack: the split can never outnumber the system
        if(bus_pv.size() + bus_pq.size() > static_cast<std::size_t>(nb_solver)) return false;
-        // a snapshot that does not cover the grid means "retired", see last_bus_status
-        if(last_bus_status.size() != nb_bus_grid) return false;
+        // built for another grid, or retired: see built_for_nb_bus
+        if(built_for_nb_bus != nb_bus_grid) return false;
        return true;
    }
 };


I thought the solver sode cache would keep a "hash", a size_t, increased by 1 each time there is a "0->1" or "1->0" (and reset to 0 each time the cache is manually reset) and this "hash" would identify uniquely the topology. Either it's the same as the grid last seen and in this case nothing needs to be done, or this is not (something has been updated) and in the case -> rebuild

BDonnot · 2026-08-31T11:52:59Z

+                         << "re-initializing this container).";
+                    throw std::runtime_error(exc_.str());
+                }
+                int sub_id = subid_(el_id);


const int maybe

BDonnot · 2026-08-31T11:54:38Z

+                // ONE bracket for the whole entry: reactivating an element and then
+                // moving it is a single change of which bus it holds, not two.
+                _apply_and_track_buses(el_id, substations, solver_control, [&]{
+                    res[el_id] = update_topo_one_el_no_bus_tracking(el_id, has_changed,


Why no_bus_tracking here ?

BDonnot · 2026-08-31T18:49:51Z

 )mydelimiter";

+const std::string DocLSGrid::deactivate_bus = R"mydelimiter(
+    .. deprecated:: 1.0.0


BDonnot · 2026-08-31T18:50:02Z

+)mydelimiter";
+
+const std::string DocLSGrid::reactivate_bus = R"mydelimiter(
+    .. deprecated:: 1.0.0


BDonnot · 2026-08-31T18:50:23Z

    raw per-bus vector, together with :func:`get_bus_vn_kv`, is the only way to inspect bus-level
    state directly.

+    .. versionchanged:: 1.0.0


Review asked for change_bus to be tested with -1, for every element and not just the two-sided ones. It cannot succeed -- _change_bus rejects a bus id below 0 or past the last bus, and -1 is the "no bus" marker, so it is the id a caller reaches for meaning "disconnect this". What the test found is where that rejection comes from: inside the counting bracket, after the element's contribution has been taken away and before it is put back. The -1 stood alone. A call the grid refused left every bus that element held one short, silently, for the rest of the grid's life. _apply_and_track_buses now restores the contribution on the way out of an exception as well as on the normal path. contribute_to_buses reads the element as it is at that moment, so it restores what the element holds after whatever did happen -- which is what a recount would say either way, whether the mutation was refused outright or gave up part way. Covered for all twelve change_bus entry points against both a negative and a past-the-end bus id: 56 assertions fail without the fix. Review also asked for synch_status_both_side at its non-default value. HvdcLineContainer hard-codes false in its constructor, so the sweep reached that path for HVDC only; a LINE or TRANSFORMER allowed to go half-open was never exercised. The whole mutator sweep now runs a second time with it off, plus the update_topo scenario, where the right answer inverts: with it on, opening one side drags the other off and the bus the live side was alone on leaves the system; with it off the branch stays connected_global and that bus stays. Same bracket, same counting rule, opposite outcome. And the four version markers this PR adds are 1.0.1, not 1.0.0 -- 1.0.0 shipped on 2026-08-28. Signed-off-by: DONNOT Benjamin <benjamin.donnot@rte-france.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E8DwbumaiafbKSb53BCodR

BDonnot

Ok

One conflict, in TwoSidesContainer_rxh_A.hpp: 3ae9427 had switched reconnect_connected_buses to get_bus_side_*_internal, and #188 deletes that function outright. Resolved in #188's favour -- the function is gone, so the _internal edit to it is moot. The other 26 _internal call sites, the branch flow arithmetic on parts, and the NDEBUG gates in fillYbus and compute_results_tsc_rxha_no_amps all survive the merge untouched. 227 test cases / 590,769 assertions pass in the C++17, C++14 and Debug builds. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013nEocx4vydkgBNQKNzRfwy

_apply_and_track_buses brackets a mutation between "take this element's contribution away" and "put it back". The bus-id validation lived inside that bracket, in _generic_change_bus, so a call the grid was going to refuse was rejected with the contribution already removed -- which is why it needed a try / catch(...) putting it back on the way out. That catch cost far more than the path it protected. An unwind edge through GenericContainer.hpp made GCC keep every std::vector<bool> access in fillYbus live across it, in a function that never calls any of this. Bisecting #188's nine commits against that one number found it exactly: eight at 7,463,013 and the ninth at 8,859,264, and deleting only the catch while keeping everything else in that commit restored 7,463,013 to the instruction. Checking first is both simpler and cheaper. GenericContainer::_check_new_bus_id is called by the four change_bus entry points before they enter the bracket, so a refused call never touches the counts at all rather than touching them and undoing it with a restore that has to reason about a half-applied mutation. It is always active: the id comes from the caller. -3,899,955 instructions on a case9241pegase rebuild solve (-0.45% of everything), of which the whole of the branch fillYbus' share: 12,085,107 -> 8,185,194, -32.3%. pre_process_solver 33,503,851 -> 29,603,932. An exception from deeper inside a mutation can still leave the counts short. That is deliberate: such a grid must be rebuilt and its caches dropped, not carried on with. #188's own coverage caught the first attempt at this, where the check had landed inside the lambda instead of before the bracket on the HVDC and two-sided paths -- 36 failing assertions, all of them the refused-change_bus cases that commit added. All 227 test cases / 590,769 assertions pass in the C++17, C++14 and Debug builds; results bit-identical on case118 and the three PEGASE cases across 16 AC and 8 DC configurations. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013nEocx4vydkgBNQKNzRfwy

Validating the bus id before the bracket removed the biggest unwind edge through GenericContainer.hpp, but not all of them. deactivate_no_bus_tracking, reactivate_no_bus_tracking and change_bus_no_bus_tracking each re-checked the element id, and _generic_deactivate / _generic_reactivate / _generic_change_bus checked it again underneath them -- three layers of the same check, only the outermost of which a user can reach. The inner two are now _check_in_range_internal, the debug-only form. Every one of the 18 _apply_and_track_buses call sites is reached through a public entry point that has already raised for a bad id, and a throw from inside the bracket is precisely what this layer exists to avoid. That takes the branch fillYbus back to exactly what it cost before #188 landed -- 7,222,254 instructions, the same figure to the digit. Over both commits: 12,085,107 -> 7,222,254 on that function (-40.2%), pre_process_solver 33,503,851 -> 28,640,988, and -4,862,863 on a whole case9241pegase rebuild solve (-0.56%). No user-facing check was lost, verified from a release build rather than by reading: it still raises for change_bus_load(0, -1), change_bus_load(0, nb+1000), the generator equivalents, change_bus_load(999999, 0) and deactivate_load(999999) -- six out of six. The Debug library carries all ten internal messages, the Release library the four user-facing ones. Two stale comments went with it: GeneratorContainer::_change_bus and SvcContainer::_change_bus both claimed their IndexError came from _generic_change_bus "which the caller runs *after* this function". change_bus_no_bus_tracking raises first, and has for some time. 227 test cases / 590,769 assertions pass in the C++17, C++14 and Debug builds -- including #188's refused-change_bus sweep, which is what makes this safe to do. Results bit-identical across 16 AC and 8 DC configurations on case118 and the three PEGASE cases. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013nEocx4vydkgBNQKNzRfwy

One conflict, in TwoSidesContainer_rxh_A.hpp: 3ae9427 had switched reconnect_connected_buses to get_bus_side_*_internal, and #188 deletes that function outright. Resolved in #188's favour -- the function is gone, so the _internal edit to it is moot. The other 26 _internal call sites, the branch flow arithmetic on parts, and the NDEBUG gates in fillYbus and compute_results_tsc_rxha_no_amps all survive the merge untouched. 227 test cases / 590,769 assertions pass in the C++17, C++14 and Debug builds. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013nEocx4vydkgBNQKNzRfwy Signed-off-by: DONNOT Benjamin <benjamin.donnot@rte-france.com>

_apply_and_track_buses brackets a mutation between "take this element's contribution away" and "put it back". The bus-id validation lived inside that bracket, in _generic_change_bus, so a call the grid was going to refuse was rejected with the contribution already removed -- which is why it needed a try / catch(...) putting it back on the way out. That catch cost far more than the path it protected. An unwind edge through GenericContainer.hpp made GCC keep every std::vector<bool> access in fillYbus live across it, in a function that never calls any of this. Bisecting #188's nine commits against that one number found it exactly: eight at 7,463,013 and the ninth at 8,859,264, and deleting only the catch while keeping everything else in that commit restored 7,463,013 to the instruction. Checking first is both simpler and cheaper. GenericContainer::_check_new_bus_id is called by the four change_bus entry points before they enter the bracket, so a refused call never touches the counts at all rather than touching them and undoing it with a restore that has to reason about a half-applied mutation. It is always active: the id comes from the caller. -3,899,955 instructions on a case9241pegase rebuild solve (-0.45% of everything), of which the whole of the branch fillYbus' share: 12,085,107 -> 8,185,194, -32.3%. pre_process_solver 33,503,851 -> 29,603,932. An exception from deeper inside a mutation can still leave the counts short. That is deliberate: such a grid must be rebuilt and its caches dropped, not carried on with. #188's own coverage caught the first attempt at this, where the check had landed inside the lambda instead of before the bracket on the HVDC and two-sided paths -- 36 failing assertions, all of them the refused-change_bus cases that commit added. All 227 test cases / 590,769 assertions pass in the C++17, C++14 and Debug builds; results bit-identical on case118 and the three PEGASE cases across 16 AC and 8 DC configurations. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013nEocx4vydkgBNQKNzRfwy Signed-off-by: DONNOT Benjamin <benjamin.donnot@rte-france.com>

Validating the bus id before the bracket removed the biggest unwind edge through GenericContainer.hpp, but not all of them. deactivate_no_bus_tracking, reactivate_no_bus_tracking and change_bus_no_bus_tracking each re-checked the element id, and _generic_deactivate / _generic_reactivate / _generic_change_bus checked it again underneath them -- three layers of the same check, only the outermost of which a user can reach. The inner two are now _check_in_range_internal, the debug-only form. Every one of the 18 _apply_and_track_buses call sites is reached through a public entry point that has already raised for a bad id, and a throw from inside the bracket is precisely what this layer exists to avoid. That takes the branch fillYbus back to exactly what it cost before #188 landed -- 7,222,254 instructions, the same figure to the digit. Over both commits: 12,085,107 -> 7,222,254 on that function (-40.2%), pre_process_solver 33,503,851 -> 28,640,988, and -4,862,863 on a whole case9241pegase rebuild solve (-0.56%). No user-facing check was lost, verified from a release build rather than by reading: it still raises for change_bus_load(0, -1), change_bus_load(0, nb+1000), the generator equivalents, change_bus_load(999999, 0) and deactivate_load(999999) -- six out of six. The Debug library carries all ten internal messages, the Release library the four user-facing ones. Two stale comments went with it: GeneratorContainer::_change_bus and SvcContainer::_change_bus both claimed their IndexError came from _generic_change_bus "which the caller runs *after* this function". change_bus_no_bus_tracking raises first, and has for some time. 227 test cases / 590,769 assertions pass in the C++17, C++14 and Debug builds -- including #188's refused-change_bus sweep, which is what makes this safe to do. Results bit-identical across 16 AC and 8 DC configurations on case118 and the three PEGASE cases. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013nEocx4vydkgBNQKNzRfwy Signed-off-by: DONNOT Benjamin <benjamin.donnot@rte-france.com>

BDonnot and others added 8 commits August 30, 2026 11:46

BDonnot commented Sep 1, 2026

View reviewed changes

BDonnot commented Sep 3, 2026

View reviewed changes

BDonnot merged commit bd145e4 into dev_1.0.1 Sep 3, 2026
66 of 118 checks passed

BDonnot mentioned this pull request Sep 4, 2026

Powerflow performance pass, and a cache that cannot outlive a failure #189

Merged

BDonnot mentioned this pull request Sep 6, 2026

Derive the voltage-control plan once per powerflow, in one class #191

Merged

Navigation Menu

Uh oh!

Bus connectivity IS the element counts, and nothing else - #188

Bus connectivity IS the element counts, and nothing else#188
BDonnot merged 9 commits into
dev_1.0.1from
claude/bus-element-counts

BDonnot commented Aug 30, 2026

Uh oh!

BDonnot left a comment

Uh oh!

BDonnot Aug 31, 2026

Uh oh!

BDonnot Aug 31, 2026

Uh oh!

BDonnot Aug 31, 2026

Uh oh!

BDonnot Aug 31, 2026

Uh oh!

BDonnot Aug 31, 2026

Uh oh!

BDonnot Aug 31, 2026

Uh oh!

BDonnot Aug 31, 2026

Uh oh!

BDonnot Aug 31, 2026

Uh oh!

Uh oh!

Uh oh!

BDonnot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

BDonnot commented Aug 30, 2026

The problem

The fix

Deleted rather than kept

Deprecated no-ops

Recovery, so drift cannot outlive an invalidation

⚠️ Binary format 4 -> 6

Tests

What it is worth

Verification

⚠️ One thing this PR cannot fix

Uh oh!

BDonnot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

BDonnot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant