Documentation audit: centralize and complete C++ core / bindings docstrings - #160
Open
BDonnot wants to merge 19 commits into
Open
Documentation audit: centralize and complete C++ core / bindings docstrings#160BDonnot wants to merge 19 commits into
BDonnot wants to merge 19 commits into
Conversation
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>
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.
Summary
Full audit of
src/core/help_fun_msg.hpp/.cpp(the shared docstring library) andsrc/bindings/python/*.cpp(the pybind11 bindings that consume it), ahead of the 1.0.0 release. Goal, per the request: (1) every public, actually-used method should have real documentation instead of"TODO"/ "internal, do not use", (2) documentation should live centrally inhelp_fun_msg.hpp/.cpprather than being duplicated/hand-written per binding, (3) flag/fix documentation whose content no longer matches the current code.TimerJac/LinearSolverStats/NR-policy docs,AlgoConfig, and theContingencyAnalysisLimitViolationfamily out of inline literals intohelp_fun_msg.PandaPowerConverter,AlgoControl, admittance fields,get_bus_id, and theSubstationContainer/SubstationInfoclasses (which had zero Sphinx coverage at all — added the missingautoclassdirectives).LSGrid"TODO"docstrings found while sweeping the bindings during section 4 (copy,timer_last_ac_pf/_dc_pf, slack-weight management, half-open HVDC handling, theset_*_namesfamily, etc).SvcContainer/SvcInfo): had zero centralized docs at all. Added the full family plus the bordered-formulation voltage-regulation/droop equations (rendered as MathJax) on request.Raises-section bug (LSGrid.check_grid) that an earlier, coarser verification pass had missed.LSGrid_internal_do_not_use-tagged methods: traced each to its actual C++ implementation and gave it a real docstring (per-element bus getters/setters, activation status, setpoint setters, slack designation, bulk vector getters/setters, structural setup, container constructors). The remaining 9 (bus-level primitives with no per-bus Python class, plus pure solver-debug/bookkeeping calls) keep the internal marker — itself rewritten to be accurate rather than claiming a single caller that wasn't true for all its uses.LSGridcross-references: every container attribute (LoadInfo.bus_id,GenInfo.target_p_mw, etc.) that now has a correspondingLSGridgetter/setter documented above links back to it, and vice versa; read-only result fields (res_p_mwand friends) note there's no setter. Also clarifies throughout thatbus_idfields are the gridmodel/global id, not the solver's internal numbering.Verification
Every pass was built, tested, and verified before committing: a real
pip install --no-build-isolation --no-deps -e .build, a real-import round-trip on every touched attribute (non-empty, non-"TODO"docstrings), and a nitpicky Sphinx rebuild (sphinx-build -b html -n) diffing the full (untruncated) set of unresolved-reference targets before/after each change — confirming zero new dangling references were introduced by ~350 new/edited docstrings and cross-references.Test plan
"TODO", no leftover"internal, do not use"where real docs were added)