Speed optim2 - #197
Merged
Merged
Speed optim2#197
Conversation
The single-solve audit could not see TimeSeries or ContingencyAnalysis at all: it only ever collects ac_pf / dc_pf. profile_batch is its sibling for the batch classes -- one solve per row on a fixed sparsity -- collecting compute() or the flow computations alone, per row, with the same trace for the A/B comparison (every row's voltages or flows, 17 digits). run_profile_batch.sh runs each compute phase over N rows and over one, so the fixed cost of a compute() is read apart from the marginal cost of a row; ab_test.sh and ab_wallclock.sh take DRIVER=batch, the latter also reporting the peak resident set size, since the memory side of a batch is invisible to an instruction count. summarize.py learns the batch phases, the fixed-versus-marginal table, and a ledger over the batch sources bucketed by function (they are member templates, so the column-0 definition scan of LSGrid.cpp does not apply). The baseline is recorded in the README. On case9241pegase a TimeSeries row costs a cached single solve (112M against 111M); a contingency row pays a connectivity search on top (1.7% in AC, 6.8% of a DC mask-mode row); reading the flows back is 2.6x a DC row; a fresh ContingencyAnalysis per grid2op step is ~110M of construction before its first row. Assisted-by: Claude Code (claude-fable-5-1) Claude-Session: https://claude.ai/code/session_015s9FrpqaLucTgGKJcLxCSw Signed-off-by: Benjamin Donnot <benjamin.donnot@rte-france.com>
…er row Whether a contingency splits the grid was found by a breadth-first search of the whole Ybus pattern: once per row in AC (check_invertible, inside remove_from_Ybus, its answer discarded in the masked mode) and once more per contingency up front in handle_disconnected_grid mode (_disconnected_buses, to list what it strands). Each walk allocated its vectors and its queue and took a hypot per nonzero -- 2.25M instructions per row on case9241pegase, or 6.8% of a DC row in the masked mode, for a question the base graph answers in constant time once its structure is known. BusGraph (src/core/BusGraph.hpp, header-only, no batch dependency so LSGrid::consider_only_main_component can adopt it later) runs one iterative DFS from bus 0 over the pattern of the solver-side matrix, the same |value| > 1e-8 edges the searches walked: preorder ranks and subtree sizes, so the subtree under a vertex is a contiguous rank range, and Tarjan's low-link, so the bridges are known. A contingency's removed edges are then read off its coefficient edits, subtracted in the edit's own order so the threshold test rounds the way the search would: no edge or a non-bridge is Connected, a bridge is Split with its stranded side the smaller subtree (ties keep the side of bus 0, as the labelling that started there did), listed in time proportional to its size. An N-k that takes several tree edges is Unknown and falls back to the old search on a patched copy, made only if such a contingency exists. BaseBatchSweep::_prepare_connectivity settles every contingency once per compute() -- only where the verdict is read: an AC row skips a split contingency, the masked mode strands the smaller side; a plain DC row leaves the split to the solver as before and builds no tree (the first version did, and cost that row 1-8%). remove_from_Ybus no longer decides anything, so modif_Ybus_time() no longer includes the check. A/B, KLU, every row's voltages bit-identical on all seven grids: ca_ac -1.4% to -3.8%, ca_ac_mask -2.4% to -4.7%, ca_dc_mask -5.3% to -9.0%, ca_dc within +0.5%. test_bus_graph.cpp holds the tree to the search on rings, feeders, a parallel pair, ties, a stranded root and random graphs, N-1 and N-2. Assisted-by: Claude Code (claude-fable-5-1) Claude-Session: https://claude.ai/code/session_015s9FrpqaLucTgGKJcLxCSw Signed-off-by: Benjamin Donnot <benjamin.donnot@rte-france.com>
… needs them What a row copied, per row: its injection (an nb_bus complex vector, off a row-major matrix whose row is already contiguous -- or, on a ContingencyAnalysis, the one constant vector), the layout's slack weights (the same vector every row, returned by value on the "nothing changed" path), a generator-voltage row bound to a plain vector before set_vm. _step_sbus returns a Map, the slack-weight helpers return a reference to the layout's own or to a scratch the range owns (one per range: no row allocates and no two threads share it), the gen_v row is a Map too. What a row told the algorithm, per row: set_masked_buses with an empty list and back, set_pv_pinned_buses with the full switchable list and back -- on rows that strand nothing and flip nothing. Every such call marks the mask positions dirty, and the next fill_J walks the Jacobian's nonzeros to recompute them. The loop now rests with nothing masked and every switchable bus pinned, establishes that once per range, and only a row that strands or flips a bus touches either. The timers around the Ybus hooks, compiled to nothing on a TimeSeries, no longer read the clock there; the current-limit check copies a branch's name into a violation rather than per branch. That last invariant exposed a bug the per-row re-pinning was hiding: the "n" solve of a ScenarioSweep with generator contingencies ran unpinned, its switchable buses solved as PQ, because _finish_preprocessing reset the algorithm AFTER _maybe_prepare_gen_contingency had pinned them, and a reset clears the pinning. compute() now resets the solver before the preparation hooks. Listed in the changelog as a fix. A/B, KLU, every row bit-identical on the seven grids: ts_ac -0.02% to -0.3%, ts_dc -1.1% to -3.9%, ca_ac -0.1% to -0.2%, ca_ac_mask -0.1% to -0.3%, ca_dc_mask -0.5% to -1.1%. Small, as the audit said: a row copies O(nb_bus) and solves O(nnz(L+U)). Assisted-by: Claude Code (claude-fable-5-1) Claude-Session: https://claude.ai/code/session_015s9FrpqaLucTgGKJcLxCSw Signed-off-by: Benjamin Donnot <benjamin.donnot@rte-france.com>
update_state turned the starting voltage into its angle and magnitude on
every solve: an atan2 and a hypot per bus, 2.1M instructions on
case9241pegase, 1.75% of a row -- and every row of a ContingencyAnalysis,
an InjectionSweep or a ScenarioSweep starts from the same seed.
BaseAlgo::set_start_polar_cache is the hint that the seed repeats. Told so,
the system keeps the last starting voltage it was handed and its polar form,
and a call with the same bits -- a memcmp, never a comparison of values --
copies the polar form back: exact, it was computed from those bits. The
FromSeed sweeps ask for it; a chained TimeSeries, which never repeats a
start, and a plain solve are told nothing and run the code they always ran.
Three things were measured on the way and are recorded in the README:
* caching unconditionally cost every solve that never hits +0.1% to +0.7%
(the three copies into the cache), hence the opt-in;
* reusing the system's own Va_/Vm_ when the start is its own last
solution (the chained case) was worth -0.4% to -2.9% of a row with the
same iteration counts, but drifts up to 3e-11 pu along the chain: a
warm start that is not the caller's bits is a different contract, so
declined;
* keeping the Newton loop's mismatch vector as a member instead of
allocating it per solve cost every solve +0.1% to +0.35% -- through a
member reference the loop compiles worse than with a local -- so the
allocation stays. Found by bisecting the two halves on case1354pegase.
A/B, KLU, every trace bit-identical on the seven grids: ca_ac -1.3% to
-2.6%, ca_ac_mask -1.3% to -2.7%, ts_ac and the single-solve phases +0.00%.
Assisted-by: Claude Code (claude-fable-5-1)
Claude-Session: https://claude.ai/code/session_015s9FrpqaLucTgGKJcLxCSw
Signed-off-by: Benjamin Donnot <benjamin.donnot@rte-france.com>
…t the solver's time The two A/B scripts built the core with -j$(nproc); twenty Eigen-heavy translation units at once took the machine into swap and the run was killed three times. JOBS=<n> caps it (the default is unchanged). profile_batch prints, next to the wall clock per row, the batch's own solver timer per row and its preprocessing time: when a change moves the clock, this says whether the solve moved or what the row does around it -- what settled a +1% on case9241pegase's ts_ac as +2% in an untouched solver against -25% in its surroundings (see the README's row-wise section). Assisted-by: Claude Code (claude-fable-5-1) Claude-Session: https://claude.ai/code/session_015s9FrpqaLucTgGKJcLxCSw Signed-off-by: Benjamin Donnot <benjamin.donnot@rte-france.com>
Two things the size of nb_steps x nb_bus, complex, are gone. The injection matrix (SbusPolicy::Vary::assemble) was built up front and read once, one row per step: 1.3 GB for a year of hourly rows on case9241pegase, plus a broadcast matrix for every axis the caller never set. Each row is now built into a buffer its range owns as the loop reaches it (fill_row), from the routing prepare() settles once per compute(): the same accumulation, in the same element order, with the same operations -- a real value enters as the complex (p, 0), a reactive one as my_i * (q, 0), the division by sn_mva is the same vectorized Eigen expression applied to the row -- so a row is the row of that matrix bit for bit. get_sbuses() builds the whole matrix only for whoever asks for it. The flows were computed branch by branch, each branch reading a COLUMN of the row-major voltage matrix: a strided pass over every row per branch, two heap temporaries the size of the batch per branch, and on a big grid the matrix re-read from memory once per branch. _flows_of_row computes a row's flows through every branch with the row's voltages read once, contiguously; the DC fast path's per-row magnitude is handed back by reference instead of as a fresh vector, and the per-row cache it needed goes with it. A/B, KLU, 100 rows, every trace bit for bit identical: on the clock the flows are -30% to -43% on every grid, a DC time-series row -26% to -47%, and peak memory down by the matrix (85 -> 71 MB on case9241pegase). The flows cost more INSTRUCTIONS on the small grids (+12% to +17%: a scalar loop against vectorized column operations) -- the item was memory-bound and the clock is what it was measured on. An AC time-series row on the two 9241-bus grids reads +1% on the clock with the binaries run alternately: the solver's own timer +2% on code this does not touch, its surroundings -25%, preprocessing -35%; instructions -0.2%. Recorded in the README, with the -march=native caveat on the vectorized division's scalar tail. Assisted-by: Claude Code (claude-fable-5-1) Claude-Session: https://claude.ai/code/session_015s9FrpqaLucTgGKJcLxCSw Signed-off-by: Benjamin Donnot <benjamin.donnot@rte-france.com>
…ared only when there are some ContingencyAnalysis.add_all_n1_contingencies registered a sweep one powerline at a time: one crossing into C++ per powerline, and one clear_results_only() each, which reset the solver and dropped the result buffers whether or not anything had been computed since the previous one -- n_line solver resets to register n_line contingencies. clear_results_only() is now a no-op when nothing computed or per-contingency exists (a flag raised by compute() and the two public connectivity entry points, lowered by the clears), and the wrapper hands the whole sweep to add_multiple_n1 in one call, keeping the per-contingency bookkeeping it did before. A contingency given by name is resolved through a dictionary built once, instead of a comparison against every name of the grid per element named. The README records two steps of the plan sized on the per-step construction profile and declined: a move constructor for LSGrid (the copy is 0.2% of a per-step construction, and the Python wrappers' copies are of the environment) and keeping the batch's cache across two compute() calls (85M of a 240M one-row repeat, nothing for a fresh object, and an invalidation surface -- the sparsity-changing per-compute configuration, the DC solver's cached injection -- that the flag-based cache warns about; the natural home is an update_grid() on the batch, out of scope). Assisted-by: Claude Code (claude-fable-5-1) Claude-Session: https://claude.ai/code/session_015s9FrpqaLucTgGKJcLxCSw Signed-off-by: Benjamin Donnot <benjamin.donnot@rte-france.com>
…-side results add_single_contingency dropped the C++ results (clear_results_only) but left the wrapper's own `__computed` flag raised, so a get_flows() after a registration that followed a computation reordered and indexed the previous result arrays with the new contingency list, and raised an IndexError once the list had grown. Found by the end-to-end check of the previous commit; reproduced on the wrapper before it. Both registration paths now lower the flag, and the next read recomputes. Assisted-by: Claude Code (claude-fable-5-1) Claude-Session: https://claude.ai/code/session_015s9FrpqaLucTgGKJcLxCSw Signed-off-by: Benjamin Donnot <benjamin.donnot@rte-france.com>
…d the scan The wall-clock table of the row-wise change was taken during a scheduled antivirus scan; re-timed on a quiet machine, alternating the binaries, the plain 9241 grid reads +0.3% and the fancy one +3%, all of it in the solver's own timer on code the change does not touch. Shifting glibc's placement for both binaries takes that gap to +0.3-0.6%: the injection buffer moved from inside a 15 MB matrix to a 148 KB allocation made after the solver's own, and where it lands against the solver's hot vectors is worth 3% on that grid -- either way. Recorded with the settings used. Assisted-by: Claude Code (claude-fable-5-1) Claude-Session: https://claude.ai/code/session_015s9FrpqaLucTgGKJcLxCSw Signed-off-by: Benjamin Donnot <benjamin.donnot@rte-france.com>
Signed-off-by: Benjamin DONNOT <BDonnot@users.noreply.github.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
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.
Improve the speed of the batch algorithm