N1 full compute - #140
Merged
Merged
Conversation
Signed-off-by: DONNOT Benjamin <benjamin.donnot@rte-france.com>
Signed-off-by: DONNOT Benjamin <benjamin.donnot@rte-france.com>
…nected component Signed-off-by: DONNOT Benjamin <benjamin.donnot@rte-france.com>
Signed-off-by: DONNOT Benjamin <benjamin.donnot@rte-france.com>
BDonnot
commented
Jun 25, 2026
| def _olf_provider_params(): | ||
| """Return ``{name: set(possible_values) or None}`` for OpenLoadFlow, or | ||
| ``None`` if the installed pypowsybl does not expose the metadata.""" | ||
| global _OLF_PROVIDER_PARAMS |
Collaborator
Author
There was a problem hiding this comment.
Need to find an alternative if possible
| Explicit version: operates on the passed solver / control / accumulators so it | ||
| can run concurrently (one solver per thread). The member Bbus_ is read-only here. | ||
| **/ | ||
| bool BaseBatchSolverSynch::compute_one_powerflow( |
Collaborator
Author
There was a problem hiding this comment.
This function should be const now
| return conv; | ||
| } | ||
|
|
||
| bool BaseBatchSolverSynch::warmup_solver( |
Collaborator
Author
There was a problem hiding this comment.
probably const too now
| if(better){ best_strand = strand; best_weight = weight; best_bus = bus; } | ||
| } | ||
|
|
||
| // 4) reorder slack_ids_me_ so the chosen reference is index 0 (the NR uses |
Collaborator
Author
There was a problem hiding this comment.
Mark (if not already) a comment in the NR and DC to tell that they MUST keep the ref slack as the first slack otherwise this breaks (and I don't even want to start debugging it)
| } | ||
| } | ||
| // the system matrix actually handed to the linear solver | ||
| Eigen::SparseMatrix<real_type> & sys_mat = has_mask ? masked_mat : dcYbus_noslack_; |
Collaborator
Author
There was a problem hiding this comment.
const ref here too maybe ?
Signed-off-by: DONNOT Benjamin <benjamin.donnot@rte-france.com>
Signed-off-by: DONNOT Benjamin <benjamin.donnot@rte-france.com>
Signed-off-by: DONNOT Benjamin <benjamin.donnot@rte-france.com>
Signed-off-by: DONNOT Benjamin <benjamin.donnot@rte-france.com>
Signed-off-by: DONNOT Benjamin <benjamin.donnot@rte-france.com>
Signed-off-by: DONNOT Benjamin <benjamin.donnot@rte-france.com>
Signed-off-by: DONNOT Benjamin <benjamin.donnot@rte-france.com>
Signed-off-by: DONNOT Benjamin <benjamin.donnot@rte-france.com>
Signed-off-by: DONNOT Benjamin <benjamin.donnot@rte-france.com>
Signed-off-by: DONNOT Benjamin <benjamin.donnot@rte-france.com>
Signed-off-by: DONNOT Benjamin <benjamin.donnot@rte-france.com>
Signed-off-by: DONNOT Benjamin <benjamin.donnot@rte-france.com>
… cpp side Signed-off-by: DONNOT Benjamin <benjamin.donnot@rte-france.com>
|
…security analysis Signed-off-by: DONNOT Benjamin <benjamin.donnot@rte-france.com>
BDonnot
commented
Jul 3, 2026
| participate. An earlier version of this function restricted participants to | ||
| the country hosting the most buses; that was a guess, not something OLF | ||
| actually does, and it was found to materially skew which generators absorb | ||
| the slack mismatch on real multi-country RTE grids -- removed; |
Collaborator
Author
There was a problem hiding this comment.
Try to avoid any references to RTE grid and just keep "real" grids. I note it here but there are other places like this
BDonnot
commented
Jul 3, 2026
| Real full grids never have any dangling line -- they only appear when | ||
| zooming into a sub-area with | ||
| ``network.reduce_by_ids_and_depths(..., with_boundary_lines=True)`` | ||
| (*eg* in ``reduce_and_compare.py`` / ``validate_olf_vs_lightsim.py``), |
Collaborator
Author
There was a problem hiding this comment.
Do not mention these scripts directly. They are not part of lightsim2grid (yet?)
BDonnot
commented
Jul 3, 2026
| ``df_dl`` carries two extra columns, ``boundary_bus_id`` / | ||
| ``boundary_vl_id``, used later to attach the equivalent branch + load. | ||
| """ | ||
| df_dl = net.get_dangling_lines() |
Collaborator
Author
There was a problem hiding this comment.
If using net, and not net_pu the unit might be wrong.
It's best to rely on pypowsybl per unit system rather than to perform the per unit here (also valid for SVC)
Loads MATPOWER cases (.m via matpowercaseframes, .mat via scipy, or an already-parsed mpc dict/object) directly into LSGrid from the raw bus/gen/branch matrices, without ever constructing a pandapower net. This sidesteps a known bug in pandapower's own matpower converter that mishandles grids with several generators connected to the same bus: lightsim2grid.network.init_from_matpower keeps every mpc.gen row as an independent generator, including at the reference bus (distributed slack across co-located generators). Verified against the real 70,000-bus case_ACTIVSg70k.m (1944 buses with multiple generators, up to 17 on one bus): parses in ~1.8s and converts/powerflows correctly. Signed-off-by: Claude <noreply@anthropic.com> Signed-off-by: Benjamin Donnot <benjamin.donnot@rte-france.com>
- support mpc.dcline: HVDC lines are now converted via model.init_dclines, using the same loss/sign conventions already validated by the pandapower loader (PF is negated to match lightsim2grid's "received at side 1" convention, LOSS1 * 100 -> loss_percent, LOSS0 -> loss_mw). Optional: falls back to no HVDC line when mpc.dcline is absent. - remove the n_sub parameter: there is always exactly one substation per matpower bus. n_busbar_per_sub now accepts any value >= 1 (previously forced to 1); the extra busbar sections it creates are deactivated since matpower has no data to populate them with. - clarify the "several reference buses" warning mentions equal weighting. - add matpowercaseframes to requirements_compile_ci.txt so CI actually exercises the ".m" file loading path instead of silently skipping it. Signed-off-by: Claude <noreply@anthropic.com> Signed-off-by: Benjamin Donnot <benjamin.donnot@rte-france.com>
…-5eoaqm Add native from_matpower initializer for LSGrid Signed-off-by: Benjamin DONNOT <BDonnot@users.noreply.github.com>
…-5eoaqm Add native from_matpower initializer for LSGrid Signed-off-by: DONNOT Benjamin <benjamin.donnot@rte-france.com>
Builds an LSGrid from a single PFΔ (arXiv:2510.22048) dataset row, a PowerModels.jl/MATPOWER-format dict with a solved AC power-flow state attached. Implemented as a thin translation layer on top of the new init_from_matpower loader: the row's PowerModels dict is converted into a raw MATPOWER-shaped bus/gen/branch array triple and delegated to it, reusing its bus-id remapping, line/transformer splitting, generator/load/shunt conversion and slack-bus handling rather than duplicating that logic. Since huggingface.co was unreachable to fetch a real sample row, the PFΔ/PowerModels schema and unit conventions were instead verified directly against the pfdelta and PowerModels.jl source repositories, and the test fixture is an independent pandapower solve of case14 translated into the verified PFΔ schema, used to check that lightsim2grid's own AC powerflow reproduces the row's solved vm/va/pf/qf/pt/qt to solver tolerance. Signed-off-by: Claude <noreply@anthropic.com> Signed-off-by: Benjamin Donnot <benjamin.donnot@rte-france.com>
Builds an LSGrid from a single PFΔ (arXiv:2510.22048) dataset row, a PowerModels.jl/MATPOWER-format dict with a solved AC power-flow state attached. Implemented as a thin translation layer on top of the new init_from_matpower loader: the row's PowerModels dict is converted into a raw MATPOWER-shaped bus/gen/branch array triple and delegated to it, reusing its bus-id remapping, line/transformer splitting, generator/load/shunt conversion and slack-bus handling rather than duplicating that logic. Since huggingface.co was unreachable to fetch a real sample row, the PFΔ/PowerModels schema and unit conventions were instead verified directly against the pfdelta and PowerModels.jl source repositories, and the test fixture is an independent pandapower solve of case14 translated into the verified PFΔ schema, used to check that lightsim2grid's own AC powerflow reproduces the row's solved vm/va/pf/qf/pt/qt to solver tolerance. Signed-off-by: Claude <noreply@anthropic.com> Signed-off-by: Benjamin Donnot <benjamin.donnot@rte-france.com>
- from_matpower.init dropped its n_sub parameter (there is always exactly one substation per matpower/PowerModels bus); mirror that in init_from_pf_delta and drop the pass-through argument. - debug_gen_pf_delta_fixture.py now solves with pandapower's trafo_model="pi" instead of its default "t" (T-equivalent) model: the two only coincide when a transformer has no iron losses / series resistance, which happens to be true for case14's transformers but is not true for MATPOWER-derived cases in general, so "pi" is the representative choice for a PFΔ-schema fixture. The committed pf_delta_case14.json fixture is byte-identical after regenerating with this flag (case14's transformers have zero iron loss / resistance), so no fixture update was needed, but the script now matches MATPOWER's convention explicitly rather than by coincidence. Signed-off-by: Claude <noreply@anthropic.com> Signed-off-by: Benjamin Donnot <benjamin.donnot@rte-france.com>
The existing CMake standard auto-detection always picks the newest standard the compiler supports, so neither end of the C++14-C++23 range lightsim2grid claims to support was ever actually exercised by CI. Add a LS2G_CXX_STANDARD CMake cache variable (root CMakeLists.txt, src/core, src/tests) to force a specific standard, and a new cpp-standards.yml GitHub Actions workflow that builds + runs the C++ Catch2 suite and the python bindings twice, once pinned to C++14 and once to C++23. Signed-off-by: Claude <noreply@anthropic.com>
The three CMake auto-detection cascades disagreed with each other: src/tests/CMakeLists.txt tried C++26 first, the other two stopped at C++23, and src/core/CMakeLists.txt additionally skipped C++20. All three now try the same sequence, C++26 down to C++14. The cpp-standards CI workflow now pins its "latest claimed" job to C++26 instead of C++23, installing g++-14 and upgrading CMake (>= 3.30 is required for CMake to even recognize C++26 support) since ubuntu-latest's default toolchain doesn't reliably provide either. Also updates the README's "tests performed automatically" section to mention the C++14/C++26 CI pin and the new upper bound. Signed-off-by: Claude <noreply@anthropic.com>
Add GitHub Actions CI for C++14 (oldest) and C++23 (latest) standards
Read-only audit of src/core/help_fun_msg.hpp/.cpp and src/bindings/python/* against the current implementation: where docs live only in binding files instead of help_fun_msg, which "TODO"/"internal" placeholders cover methods actually used by the lightsim2grid Python package, and concrete cases where docstring content no longer matches the code (renamed enum values, swapped Doc constants, stale Jacobian/DC-slack descriptions, a broken example). No code changes. Signed-off-by: Claude <noreply@anthropic.com>
- Fix swapped Doc constants: AlgorithmSelector.get_error, LSGrid/ ContingencyAnalysis.available_default_algorithms, HvdcLineInfo.p2_mw, SubstationInfo.nb_max_busbars/vn_kv (new DocIterator entries added). - Clean up 5 raw-string docstrings in binding_batch.cpp that had leftover quote characters baked into the compiled docstring text. - Update all 20 DocSolver solver-family descriptions (and their usage examples) to the current AlgorithmType enum names (NR_/NRSing_/DC_/ FDPF_XB_/FDPF_BX_ prefixes), and fix the FDPF_BX_* entries mislabeling themselves "XB version". - Point DocComputers/DocSecurityAnalysis cross-references at the current lightsim2grid.timeSerie.TimeSeriesCPP / lightsim2grid.contingencyAnalysis. ContingencyAnalysis(CPP) names instead of the deprecated timeSerie.Computers / securityAnalysis.SecurityAnalysis(CPP) ones. - Fix get_power_flows' self-contradictory kA/MW units, and get_error's leftover "TODO DOC" placeholder (now documents ErrorType properly). - Rewrite J_description for the current composable NRSystem<Base, MultiSlack, VoltageControl, Hvdc> architecture instead of the old fixed 2x2 + single slack row/column shape, and drop the now-false "ref buses are pv except the first, cannot be changed" claim. - Fix get_slack_ids_dc(_solver) claiming distributed slack is ignored in DC (it isn't; only get_ptdf/get_lodf still assume a single slack bus). - Fix get_timers/get_computation_time referencing a nonexistent timer_total_ field (it's timer_total_nr_), and bus_ex_id saying "lv" instead of "ex". - Rewrite the hvdc-line docs (DCLineContainer/DCLineInfo/dc_line_formula) for the current IIDM converter-station + angle-droop model instead of the retired pandapower 2-generator model, fix the broken bus_or_id example, resolve the load/generator sign-convention contradiction, split the shared/misleading p2_mw doc into its own side-2 entry, and drop the dead gen_or/gen_ex constants describing a removed API. - Rewrite min_p_mw/max_p_mw and min_q_mvar/max_q_mvar to accurately describe their actual (very different) usage per element type. - Also fix is_slack referencing the renamed SparseLUSingleSlack class. DOCUMENTATION_AUDIT.md updated to mark all 18 section-3 findings as fixed. Signed-off-by: Claude <noreply@anthropic.com>
…c docs The earlier hvdc-doc rewrite only fixed the hvdc-specific DocIterator entries; LineInfo's own fields (bus_or_id/bus_ex_id, res_p_or_mw/res_p_ex_mw, res_q_or_mvar/res_q_ex_mvar, res_v_or_kv/res_v_ex_kv, res_a_or_ka/res_a_ex_ka, res_theta_or_deg/res_theta_ex_deg) and the shared line_model ASCII schema (used by r_pu/x_pu/h_pu for both lines and transformers) still used the retired "or"/"ex" (origin/extremity) naming, even though the actual bound Python attributes are bus1_id/bus2_id, res_p1_mw/res_p2_mw, etc. Renamed every affected DocIterator constant to the _1_/_2_ convention (including the hvdc _dcline-suffixed ones, for naming consistency) and rewrote the schema to use bus 1/bus 2, i1/i2, v1/v2, with a note that transformers keep hv/lv (a real electrical distinction, left untouched). DOCUMENTATION_AUDIT.md updated with this follow-up finding, marked fixed. Signed-off-by: Claude <noreply@anthropic.com>
TwoSidesContainer_rxh_A::compute_yac builds yac_11 = ys + h1, yac_22 = ys + h2
from two independent per-side values (h1_pu/h2_pu) -- not one shared "h"
split symmetrically in half at each side, which is what the schema's old
"1/2*h" label at both terminals claimed. Replaced it with distinct h1/h2
labels plus a note that they can legitimately differ (eg an asymmetric
line/transformer imported from pypowsybl).
Also fixed h_pu's own text, which had conductance/susceptance backwards
("capacitance (real part) and dielectric conductance (imaginary part)"):
DataConverter.cpp builds h = g + 1j*b, so conductance is the real part and
susceptance (line-charging capacitance) is the imaginary part -- confirmed
independently by h_side_1_'s imaginary part feeding the FDPF B'' matrix, a
susceptance-only quantity.
DOCUMENTATION_AUDIT.md updated with this as finding 20.
Signed-off-by: Claude <noreply@anthropic.com>
Actually built the C++ extension and ran sphinx-build (not just syntax-checked one file in isolation): 0 warnings, 0 errors on the project's real build, both before and after the section-3 fixes. Also ran a stricter nitpicky pass to hunt for dangling cross-references specifically. Confirmed the 1537 resulting warnings are a pre-existing, systemic baseline (KLU/NICSLU/CKTSO classes uncompiled here, pybind11 type stubs, and -- newly discovered, out of scope for this pass -- every lightsim2grid.solver.* reference in help_fun_msg.cpp being unresolved since that module is a deprecated alias not exposed to autodoc), not something introduced by the fixes. One nitpicky-only warning did shift as a side effect of the binding_batch.cpp quote cleanup; verified via a revert-and-rebuild that the underlying numpydoc quirk itself is pre-existing and invisible in the real, non-nitpicky build. Signed-off-by: Claude <noreply@anthropic.com>
…ion)
Solvers.hpp aliases NR_KLU/NR_NICSLU/NR_CKTSO (and their DC/FDPF/NRSing/
NRRefactorRetry siblings) to the SparseLU family's own C++ types when the
real library isn't available and _READ_THE_DOCS is set -- they become the
literal same type, not a distinct one. But binding_solvers.cpp still tried
to register each alias as its own py::class_<>, which pybind11 forbids for
a type that's already bound under another name: importing a module built
with _READ_THE_DOCS (and none of KLU/NICSLU/CKTSO actually available)
raised "generic_type: type X is already registered!" and crashed on
import, defeating the flag's purpose.
Restructured each of the three solver-family blocks to bind normally when
the real library is available, and fall back to plain Python-level
attribute aliases (m.attr("NR_KLU") = m.attr("NR_SparseLU")) when only
_READ_THE_DOCS is set -- these just point the name at the already-bound
SparseLU class object instead of re-registering its C++ type, which is
what Sphinx autodoc needs to resolve :class: references to these names
without a real KLU/NICSLU/CKTSO build.
Signed-off-by: Claude <noreply@anthropic.com>
Documents the pybind11 double-registration crash found while trying to use _READ_THE_DOCS for a complete nitpicky Sphinx pass, the fix (attribute aliases instead of re-registering an already-bound C++ type), and the verification: real import now works with _READ_THE_DOCS set (NR_KLU is NR_SparseLU, etc.), the normal build path is unaffected (NR_KLU correctly stays unbound without the flag), and nitpicky warnings dropped 1537 -> 1483 with all algorithm_names.rst KLU/NICSLU/CKTSO "class not found" warnings resolved. Signed-off-by: Claude <noreply@anthropic.com>
…liases
NR_KLU/NR_NICSLU/NR_CKTSO (and their DC_*/FDPF_*/NRSing_*/NRRefactorRetry_*
siblings) previously aliased NR_SparseLU's C++ type under _READ_THE_DOCS,
which crashed pybind11 registration on import ("type already registered")
and, once patched around with attribute aliases, defeated the point of
_READ_THE_DOCS by giving each solver the same name, docstring, and methods
as NR_SparseLU instead of its own.
Each optional linear-solver header (KLUSolver.hpp/NICSLUSolver.hpp/
CKTSOSolver.hpp) now defines a header-only, no-op-bodied stand-in class of
the same name and public interface as the real one, active only under
_READ_THE_DOCS when the real library isn't. This makes every NR_KLU-style
type genuinely distinct, correctly named, and independently documented,
so binding_solvers.cpp needs no _READ_THE_DOCS special-casing at all.
Also extends SolverInstantiations.cpp's explicit template instantiations
(needed for BaseFDPFAlgo::fillBp_Bpp, defined out-of-line) and Solvers.hpp's
extern template declarations to cover _READ_THE_DOCS, and propagates the
_READ_THE_DOCS compile definition to the lightsim2grid_core CMake target
(previously only reached the bindings target), fixing an undefined-symbol
link error at import time.
Verified: _READ_THE_DOCS=1 build imports cleanly, NR_KLU is a distinct
type with its own doc/name and working no-op methods; the normal build
(no _READ_THE_DOCS) is unaffected; nitpicky Sphinx build shows no new
warnings beyond the pre-existing lightsim2grid.solver.* cross-reference
issue that already affects NR_SparseLU.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CyjDW1GbiuZY3AsHWcsCuZ
Signed-off-by: Claude <noreply@anthropic.com>
…algorithm) lightsim2grid.solver is a deprecated shim re-exporting lightsim2grid.algorithm, but 121 :class:/:attr:/:func: references in help_fun_msg.cpp (99) and binding_enums.cpp (22) still pointed at the old lightsim2grid.solver.* path, which Sphinx can't resolve since these classes are documented under lightsim2grid.algorithm (docs/solvers.rst automodules that path). Same "rename not fully propagated" pattern as the earlier or/ex->1/2 and shunt-admittance findings, just for solver->algorithm. Also fixes an adjacent bug: DocSolver::NRSing_SparseLU referenced :class:`lightsim2grid.algorithm.AlgorithmType.NR_KLU` (an enum member, wrong path) where the parallel sentence everywhere else references the solver class directly. Verified via nitpicky Sphinx rebuild: lightsim2grid.solver.* warnings 121 -> 0, total warnings 2352 -> 2166, no new warnings introduced. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CyjDW1GbiuZY3AsHWcsCuZ Signed-off-by: Claude <noreply@anthropic.com>
Add intersphinx_mapping (python/numpy/scipy/pandas/grid2op/pypowsybl) to docs/conf.py -- it was never set despite sphinx.ext.intersphinx being loaded, so every third-party type hint in a docstring (numpy.typing.NDArray, scipy.sparse.csc_matrix, grid2op.Environment.Environment, ...) was unresolvable. Accounts for the large majority of nitpicky warnings. Also fixes three genuine docstring bugs surfaced while triaging the remaining warnings: - lightsim2grid.LightSimBackend.LightSimBackend -> lightsim2grid.lightSimBackend.LightSimBackend (wrong module path, in a boilerplate string shared by ~150+ methods). - The three get_J-related passages claimed a gridmodel-labelled LSGrid.get_J exists alongside get_J_solver, mirroring get_Va/get_Vm. It doesn't -- only get_J_solver was ever added. Reworded to state the actual (asymmetric) behavior instead of a dangling reference. - get_timers_jacobian referenced with no module path; fully qualified. Verified via nitpicky Sphinx rebuild: the three docstring bugs go to 0 occurrences each; total warnings 2166 -> 1970. The intersphinx inventories themselves can't be fetched in this sandbox (network policy blocks numpy.org/scipy.org/etc., confirmed via ProxyError 403 in the build log), so that part of the fix is unverified here but should resolve on Read the Docs' real build servers. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CyjDW1GbiuZY3AsHWcsCuZ Signed-off-by: Claude <noreply@anthropic.com>
…station iterator fields
Renames the two remaining stale doc-struct names to match their actual
classes: DocComputers -> DocTimeSeries (TimeSeries/TimeSeriesCPP),
DocSecurityAnalysis -> DocContingencyAnalysis (ContingencyAnalysis/
ContingencyAnalysisCPP), including their class-doc fields (Computers ->
TimeSeries, SecurityAnalysis -> ContingencyAnalysis).
Fixes the section-4 "missing docs" gaps on GenInfo, HvdcLineInfo and
ConverterStationInfo -- the containers most recently reworked. Every field
was already bound to Python; the gap was purely documentation:
- GenInfo.voltage_regulator_on / .target_q_mvar were literal "TODO"
(target_q_mvar was a wiring bug: the real, generic DocIterator entry
already existed, just never used here).
- GenInfo.regulated_bus_id ("remote voltage control") had a one-line inline
string, not centralized, missing the voltage_regulator_on interplay and
the pypowsybl-import caveat already documented in docs/network.rst prose.
- ConverterStationInfo (one per hvdc line side) had never been added to
DocIterator at all: class doc + converter_type, loss_factor,
voltage_regulator_on (TODO), target_q_mvar (TODO), power_factor were all
inline/terse.
- HvdcLineInfo's newer IIDM/droop fields (converters_mode, p_setpoint_mw,
r_ohm, nominal_v_kv, droop_enabled, droop_p0_mw, droop_k_mw_per_rad,
pmax_1to2_mw, pmax_2to1_mw, status_droop, station1/station2) were all
inline one-liners.
Adds 19 new DocIterator entries and wires all of the above bindings to
them. voltage_regulator_on is one entry shared by GenInfo and
ConverterStationInfo (identical PV/PQ semantics); target_q_mvar is extended
(not duplicated) since it's now reused by 6 classes. Cross-references
inside shared strings are always fully qualified
(lightsim2grid.elements.<Class>.<attr>), since a bare :attr: role only
resolves against the current page's class and these strings render on
multiple classes' pages.
Also fixes a second pre-existing "rename not fully propagated" bug of the
same kind as the earlier solver->algorithm one, surfaced while wiring
ConverterStationInfo's shared min/max_q_mvar docs: 14 references to the
deprecated lightsim2grid.gridmodel module (should be lightsim2grid.network),
8 of which were additionally missing the LSGrid. class segment entirely.
Verified via nitpicky Sphinx rebuild: fixed three new dangling refs caused
by a pre-existing numpydoc quirk (colon + comma-list at the start of a
docstring misparsed as a type field) in three of the new strings by
rewording; 0 gridmodel/missing-LSGrid. references left; total warnings
1306 -> 1273. Spot-checked rendered HTML anchors and round-tripped the
renamed doc-struct usages through a real import.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CyjDW1GbiuZY3AsHWcsCuZ
Signed-off-by: Claude <noreply@anthropic.com>
…, get_bus_id, Substation docs
Fixes the remaining 5 cataloged section-4 items:
- PandaPowerConverter (class + 6 conversion methods): had zero docstring.
Corrected two things its own header comment got wrong: it calls itself
"provided as examples" but is actually what
lightsim2grid.network.init_from_pandapower uses for every real
pandapower grid load; and get_trafo_param_pp3 called itself "legacy"
when it's actually the newer pandapower-3 path (confirmed against the
version-gated call site in _aux_add_trafo.py). Also documents, rather
than hides, a real pre-existing implementation gap found along the way:
get_line_param's h1/h2 split is currently just a hardcoded 50/50 of the
legacy unsplit h (marked // TODO in DataConverter.cpp), not a genuine
per-side split.
- AlgoControl (class + 12 flags): were all literal "TODO". Also fixes two
adjacent "TODO"s on LSGrid.get_ac_algo_controler/get_dc_algo_controler
(the only way to reach an AlgoControl from Python), and re-exports
AlgoControl under lightsim2grid.algorithm (mirroring AlgoConfig) so it's
actually reachable and autodoc'd instead of dangling.
- TrafoInfo/LineInfo admittance fields (yac_11/12/21/22, yac_eff_11/12/21/22,
ydc_11/12/21/22, + the 4 get_yac_eff_* container methods x2): traced the
actual pi-model + Kron-reduction formulas in TwoSidesContainer_rxh_A.hpp /
TrafoContainer.cpp to document accurately, connecting them to the
line_model schema fixed earlier in this audit.
- get_bus_id / get_bus_id_side_1 / get_bus_id_side_2: were "TODO doc",
except HvdcLineContainer's two, which had no docstring argument at all.
- SubstationContainer/SubstationInfo: class docstrings were "TODO". Also
fixes LSGrid.get_substations/get_voltage_levels ("TODO", the only way to
reach a SubstationContainer), and adds the missing autoclass directives
to docs/network.rst -- both classes were referenced from prose there but
never actually documented, so those references were dangling.
Also fixes, while in the area: 9 occurrences of a fully broken code
example (from lightsim2grid.gridmodel import init / init(pp_net) --
lightsim2grid.gridmodel.init does not exist) across the get_lines/
get_trafos/get_generators/... family of LSGrid accessor docstrings.
Flags, but does not fix (out of scope for "the rest of section 4" as
cataloged): ~15 more literal "TODO" docstrings on other LSGrid methods
found while grepping the same file.
Verified via nitpicky Sphinx rebuild: caught and fixed two new dangling
references before reporting done (a bare :attr:`line_model` role -- not
itself a bound attribute -- and one more instance of the previously
documented numpydoc colon-quirk in ydc_11). Total warnings 1273 -> 1267.
Confirmed non-empty __doc__ via real import for every newly-documented
class/method; ran test_DataConverter.py (unchanged functionally) to
confirm nothing broke.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CyjDW1GbiuZY3AsHWcsCuZ
Signed-off-by: Claude <noreply@anthropic.com>
Moves every substantive inline docstring cataloged in section 2 (and a few adjacent gaps found along the way) out of the binding .cpp files and into the shared help_fun_msg.hpp/.cpp docstring library, so a second binding (C, Rust, ...) wouldn't need to duplicate them: - TimerJac (class + 13 fields) and LinearSolverStats (class + 12 fields), binding_solvers.cpp: previously had zero docstring at all (not even a placeholder). Traced the actual timer semantics through NRAlgo.tpp / BaseDCAlgo.hpp / BaseFDPFAlgo.hpp to document them accurately. - bind_nr_algo_policies (19 methods), bind_linear_solver_stats (1), bind_fdpf_linear_solver_stats (2): moved as-is, content was already good. - AlgoConfig (class + int_params/real_params), binding_misc.cpp: moved as-is, including the mutate-by-reassignment warning. - LimitViolation/ViolationElementType/LimitViolationType (class + 7 fields), binding_batch.cpp: moved as-is. - ~55 LSGrid methods/properties in binding_lsgrid.cpp (more than the audit's original "~40" estimate): every substantive inline docstring in the file -- underscore-prefixed bookkeeping properties, algo-config accessors, per-bus voltage limits, slack/PV-PQ bookkeeping, names, every half-open (per-side) connect/disconnect method, transformer tap/ phase-shift, remote voltage control, HVDC angle-droop, and the whole family of solver-internal-state getters used by external solvers re-deriving the NR system. Left ~10 short "DEPRECATED: use X instead" redirects inline (not worth centralizing). Also merges a small pre-existing dead constant found along the way: DocLSGrid::available_algorithm_names already had real content but was never wired to any binding. Verified via nitpicky Sphinx rebuild after each batch: caught and fixed several new dangling references before calling it done (bare :func:/:attr: roles that don't resolve on the class page they actually render on -- eg get_timers_jacobian is only ever bound on AlgorithmSelector, not on individual solver classes), plus more instances of the previously documented numpydoc colon-quirk. Final diff of unresolved-reference targets before/after this pass is empty: zero new dangling-reference categories despite touching ~90 docstrings. Round-tripped ~52 of the newly-centralized attributes through a real import to confirm non-empty docstrings. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CyjDW1GbiuZY3AsHWcsCuZ Signed-off-by: Claude <noreply@anthropic.com>
…pass Documents copy, timer_last_ac_pf/timer_last_dc_pf, get_turnedoff_gen_pv, update_slack_weights(_by_id), assign_slack_to_most_connected, consider_only_main_component, get_ignore_status_global/ get_synch_status_both_side, the set_*_names family (line/dcline/trafo/ gen/load/storage/sgen/shunt/svc), and change_ratio_trafo -- all previously literal "TODO" placeholders on LSGrid, flagged but not fixed during the section-4 pass. Traced each one's actual implementation (LSGrid.cpp/.hpp, GeneratorContainer.cpp) rather than guessing from the name. Notably, consider_only_main_component's C++ comment carried a stale "// TODO DC LINE: one side might be in the connected comp and not the other !" -- traced the call chain and confirmed this is already handled by HvdcLineContainer::disconnect_if_not_in_main_component (documented in the earlier Gen/HVDC pass): documented the current, correct behavior instead of perpetuating the stale warning. Dropped two candidate cross-references (GeneratorContainer::add_slackbus / remove_all_slackbus) after confirming they're real C++ methods but never bound to Python at all -- referencing them would have been dangling. Verified via nitpicky Sphinx rebuild: the diffed set of unresolved- reference targets is byte-for-byte identical before and after this pass. Round-tripped all 20 touched attributes through a real import to confirm non-empty, non-"TODO" docstrings. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CyjDW1GbiuZY3AsHWcsCuZ Signed-off-by: Claude <noreply@anthropic.com>
…quations Move SvcContainer/SvcInfo/RegulationMode and the 13 SvcInfo field docstrings out of inline literals in binding_containers.cpp into help_fun_msg.hpp/.cpp (the last container family left undocumented in the centralized doc store), and add the bordered-formulation voltage-regulation equations (including the droop/slope term and the pypowsybl slope-unit conversion) to SvcContainer's docstring as rendered math. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CyjDW1GbiuZY3AsHWcsCuZ Signed-off-by: Claude <noreply@anthropic.com>
Full (untruncated) unresolved-reference diffing, prompted by the SVC pass, surfaced 8 docstrings across earlier audit commits and pre-existing code where a prose colon got misparsed by numpydoc as a name/type or Raises field and its "type" text cross-referenced: AlgoConfig.int_params/ real_params, LimitViolation.element_id/name, ContingencyAnalysisCPP. compute_limit_violations/nb_thread, and LSGrid.check_grid's Raises section (reformatted into proper numpydoc Raises/Returns fields). Also fixes an unrelated network_factory type-field misuse in _olf_compare.py and an Opional/Optional typo in initLSGrid.py, both caught by the same rebuild. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CyjDW1GbiuZY3AsHWcsCuZ Signed-off-by: Claude <noreply@anthropic.com>
… marker Trace each _internal_do_not_use-tagged LSGrid method (binding_lsgrid.cpp) to its actual container delegation and give it a real, centralized docstring in help_fun_msg.hpp/.cpp: per-element bus getters/setters, activation status, setpoint setters, slack designation, bulk whole-container vector getters/setters, structural position/subid setters, and bulk container constructors, plus grid-level scalars (sn_mva, init_vm_pu, n_sub) and the unset_changes/tell_solver_need_reset solver-bookkeeping pair. The remaining 9 (bus-level primitives with no per-bus python class, and pure solver-debug/bookkeeping calls) keep the internal marker, itself rewritten to be generic and honest rather than claiming a specific LightSimBackend-only caller that isn't accurate for the solver and ContingencyAnalysis bindings that also use it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CyjDW1GbiuZY3AsHWcsCuZ Signed-off-by: Claude <noreply@anthropic.com>
…etters/setters Every container attribute (bus_id, connected, target_p_mw, ...) that has a corresponding LSGrid getter/setter now points back at it, and vice versa: read-only here, equivalent to LSGrid.get_X(), change via LSGrid.change_X(). Result-only fields (res_p_mw and friends) get a "no setter, see the bulk getter" note instead. Since many of these docstrings were previously a single DocIterator constant shared verbatim across several container classes, give each (container, field) pair needing a distinct LSGrid cross-reference its own self-contained constant instead of concatenating a shared base with a per-site suffix -- no runtime pairing to get wrong at future binding call sites. Fields with no LSGrid-level counterpart (ConverterStationInfo's, which follow their parent HvdcLineInfo) keep the original shared constant, extended with a note saying so. Also clarify throughout that bus_id fields are the gridmodel/global bus id, not the solver's internal bus numbering, and delete the now-orphaned bus_1_id/bus_2_id constants superseded by per-container ones. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CyjDW1GbiuZY3AsHWcsCuZ Signed-off-by: Claude <noreply@anthropic.com>
The audit write-up isn't meant to ship as a tracked file in the repo. Signed-off-by: Claude <noreply@anthropic.com>
…2rq0y Documentation audit: centralize and complete C++ core / bindings docstrings
…e branch Follow-up security audit in the spirit of PRs #155/#156/#157, over the channels docs/security.rst covers (python bindings, pickle, the binary format) plus the new ContingencyAnalysis (n-1) code path. Four issues, each reproduced as a segfault / heap corruption before the fix and as a clean exception (or safe behaviour) after. Release wheels are -O3 -DNDEBUG, so neither Eigen's nor the standard library's own bounds checks catch any of these. * ContingencyAnalysis stale results -> out-of-bounds write. compute() sizes the result matrices (_voltages and the flow matrices derived from it) with one row per registered contingency. Adding / removing contingencies afterwards, then calling compute_flows() / compute_power_flows() without recomputing, left clean_flows() (and the per-contingency violation loop) walking the new, longer _li_defaults while indexing the old, shorter matrices -- a plain `ca.add_n1(...)` after compute() corrupted the heap ("free(): invalid pointer"). compute_flows() / compute_power_flows() now refuse the stale state with a clear message. * OneSideContainer::update_topo -> out-of-bounds read. has_changed / new_values are indexed by the stored pos_topo_vect with an unchecked Eigen operator(). check_grid() proves those positions form a permutation of [0, dim_topo), but the set_*_pos_topo_vect() setters only length-check their argument, not its values -- so a position written straight through a setter (bypassing check_grid) read past the caller arrays. Validate el_pos against has_changed.rows() before indexing, same idiom as the entry-path checks added in #143. * check_grid accepted vmin/vmax with mismatched presence -> out-of-bounds read. bus_vmin_kv_ / bus_vmax_kv_ are each individually optional (empty-or-complete), but ContingencyAnalysis::check_bus_voltage_violations consumes them together: it loops to vmin.size() and then reads vmax(grid_id). A state with one set and the other empty passed every per-field check yet over-read the empty one (reachable from a pickle / a binary file). check_grid now requires them to have the same presence. * LSGrid sub-object getters -> use-after-free. get_lines / get_generators / get_substations / ... and the Eigen-view getters get_bus_vn_kv / get_V_solver / ... were bound with return_value_policy::reference, which does NOT keep the parent grid alive. Dropping the grid while holding the returned container / numpy view left it dangling: reading through it disclosed freed heap (a 53M-element "bus id" array) and segfaulted. Switch the getters that return a reference / view into grid-owned memory to reference_internal so their lifetime is tied to the grid. Same fix for AlgorithmSelector::get_fdpf_{xb,bx}_lu. Tests: 13 new python cases in test_state_poisoning.py and a new test_check_grid.cpp case for the vmin/vmax presence check. Full C++ suite (128 tests) and the python poisoning suite pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vxxj4GukTFz1TKh52GZxLP Signed-off-by: Claude <noreply@anthropic.com>
…licitly Add a subsection making the core design rule the previous audits enforce explicit for readers: whatever an LSGrid is handed -- a malformed pickle / binary file, a grid from a source file, or an ordinary API call in the wrong order or with an out-of-range id -- the worst it may do is raise a clean Python exception, never read/write out of bounds, use freed memory, or leave the grid in a state that makes the next call do so. Notes that this is about safety (not answer-correctness) and that a genuine crash instead of an exception is a bug to report. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vxxj4GukTFz1TKh52GZxLP Signed-off-by: Claude <noreply@anthropic.com>
Three follow-ups from review of #161, each a real (proved before/after) "silently wrong data" bug rather than a crash, closed with the same validate-eagerly philosophy as the rest of this audit. * ContingencyAnalysis: a size-only staleness check (added in the previous commit) cannot tell remove_n1(x) + add_n1(y), y != x, apart from a clean state -- both leave _li_defaults at the SAME size it was at compute() time, just with different membership. compute_flows() would then silently return flows that correspond to the wrong contingencies instead of raising (reproduced: same array, different values, no exception). Every mutating method (add_all_n1 / add_n1 / add_multiple_n1 / add_nk / remove_n1 / remove_multiple_n1 / remove_nk) now eagerly clears any previously computed results via clear_results_only(), closing the gap for any sequence of calls rather than only the ones a size comparison happens to catch. Validation runs to completion before any mutation (or clearing) happens, so a rejected id leaves both the contingency set and any existing results untouched -- the batch methods now validate every id first, then mutate, rather than interleaving the two. * OneSideContainer::set_pos_topo_vect / set_subid: only length-checked their argument, deferring all value validation to update_topo() (fixed in the previous commit) or to an explicit check_grid() call that the caller might never make. Reject a negative value immediately in the setter itself -- the only bound it can check locally, since the upper bound (dim_topo for positions, n_sub for substation ids) depends on context the setter does not have (other containers' element counts / SubstationContainer, respectively). * OneSideContainer::update_topo: a substation id feeds SubstationContainer::local_to_gridmodel's unchecked arithmetic (sub_id + (busbar - 1) * n_sub) to resolve the target bus. That function's own bound check is on the OUTPUT (the resolved bus id, which is what actually gets used downstream), not on sub_id -- so an out-of-range sub_id can land BY COINCIDENCE on another substation's legitimate bus id and silently reconnect the element to the WRONG bus rather than raising (reproduced: load reassigned to an unrelated substation's bus, no exception). Validate sub_id against substations.nb_sub() where that context is available, mirroring the el_pos check added for pos_topo_vect in the previous commit; also guard the case where subid_ was never set at all (empty), which indexed an empty vector. Tests: 7 new python cases (test_state_poisoning.py) and 4 new C++ cases (test_check_grid.cpp), all reproduced as failing before the fix and passing after. Full suites green: 132 C++ tests, 46 python tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vxxj4GukTFz1TKh52GZxLP Signed-off-by: Claude <noreply@anthropic.com>
…old, setter-does-not-validate contract The Sanitizers workflow's "ASan + UBSan tests" job failed on a5b8a5b: not a memory-safety issue (no sanitizer report -- both new C++ checks are unit-tested directly in test_check_grid.cpp and pass clean under ASan/UBSan), but a pre-existing Python test file (lightsim2grid/tests/test_check_grid.py, distinct from this audit's own test_state_poisoning.py) that predates this PR and assumed set_load_pos_topo_vect() / set_load_to_subid() / set_gen_to_subid() would silently accept a negative value and defer the rejection to a later check_grid() call. That assumption no longer holds: the previous commit made those setters reject a negative value immediately (the one bound they can check locally). This is a strictly earlier, more specific rejection of the exact same invalid input -- not a change in what is ultimately accepted -- so check_grid() never gets a chance to see the poisoned state in these two tests. Move the assertRaises to wrap the setter call instead of the check_grid() call; the "out of range but non-negative" siblings (test_out_of_range_pos_topo_vect_is_rejected, test_out_of_range_subid_is_rejected, test_subid_just_out_of_range_is_rejected) are untouched, since that bound still needs check_grid()'s cross-container context and continues to pass as before. Confirmed no other caller (production LightSimBackend / pypowsybl loader, or any other test file) relies on the old silent-acceptance behavior: grep across lightsim2grid/ for every set_*_pos_topo_vect / set_*_to_subid call site shows production code only ever passes grid2op/pypowsybl-derived, always-valid, non-negative data. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vxxj4GukTFz1TKh52GZxLP Signed-off-by: Claude <noreply@anthropic.com>
…t nb() Follow-up to the sub_id fix in a5b8a5b, prompted by a review question: that fix checked subid_.size() == 0 ("never set") but not subid_.size() != nb() ("stale"), even though the exact same class of check already exists for pos_topo_vect_ (_check_pos_topo_vect_filled()'s second condition). subid_ is only ever (re)assigned by set_subid() -- checked against nb() at the time of that call -- or by set_osc_state() (pickle / binary load). init() does not touch it. So a container re-initialized with MORE elements after set_subid() was called for the smaller count leaves subid_ shorter than the container's current nb(): indexing subid_(el_id) for an el_id past its old length reads out of bounds. Reproduced: init_loads(1 load) + set_load_to_subid + init_loads(3 loads) [reusing the same LSGrid, subid_ left at its old size 1] + set_load_pos_topo_vect(sized to 3) + update_topo() reconnecting the 3rd load -- no exception, over-read reachable. Add the missing subid_.size() != nb() check, mirrored from _check_pos_topo_vect_filled(). One new C++ test ("update_topo rejects a subid left stale after the container grew"). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vxxj4GukTFz1TKh52GZxLP Signed-off-by: Claude <noreply@anthropic.com>
Close four more memory-safety holes on the untrusted-input / public-API paths
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.
No description provided.