Reorganize docs, add info, reduce redundancies - #337
Open
acostarelli wants to merge 20 commits into
Open
Conversation
Author the tutorials and how-to guides as executable Literate .jl sources that generate Documenter-flavored markdown (+ notebooks) at build time, matching the PowerSystems.jl docs setup. Generalize the tutorial build pipeline (make_literate_folder) to cover both the tutorials/ and how_to_guides/ folders, and repoint make.jl nav at the generated_*.md outputs. Move the stray network_matrices.jl into tutorials/ and repair it to load via PowerSystemCaseBuilder. Fix API usages the old @repl blocks silently rendered as errors: qualify the unexported matrix getters (get_axes/get_lookup/get_data) with PNM, and pass irreducible_buses to the matrix constructor instead of to the reduction types. Wrap the BA/ABA expected-error demo in try/catch and keep benchmark/MKLPardiso/VirtualMODF-contingency snippets non-executed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…diataxis framework based first on how PSI uses PNM.
…nd ref, new howto, better docstrings, remove duplicative reference pages
…tter connectivity howto (maybe better for tutorial?)
Contributor
There was a problem hiding this comment.
Pull request overview
This PR restructures the documentation to reduce redundancy and shift most narrative content into Literate.jl sources, while also tightening/modernizing many in-code docstrings (notably around tolerances, reductions, solver backends, and mathematical notation).
Changes:
- Migrates several tutorials/how-to pages from static
.mdinto Literate-generated.jlsources, and updates the Documenter navigation/build pipeline accordingly. - Expands/clarifies docstrings across PTDF/LODF/virtual matrices, network reduction types, serialization behavior, and
AutoTolerance, including more LaTeX math blocks. - Adds new reference hubs/pages (e.g., matrix overview/indexing, matrix type reference) and new explanation pages (concurrency, slack conventions, equivalent branches).
Reviewed changes
Copilot reviewed 45 out of 45 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/virtual_lodf_calculations.jl | Docstring math formatting for partial LODF derivation. |
| src/serialization.jl | Clarifies PTDF-only HDF5 serialization scope and what is/isn’t persisted. |
| src/ptdf_calculations.jl | Updates PTDF constructor docs (solver list, AutoTolerance default, math notation). |
| src/NetworkReductionData.jl | Adds guidance on accessors and “bus survived reduction” checks. |
| src/NetworkReduction.jl | Adds user-facing rules and usage patterns for network_reductions ordering/validation. |
| src/modf_definitions.jl | Reformats Woodbury equation and symbol descriptions using LaTeX. |
| src/lodf_calculations.jl | Updates LODF constructor docs (solver list, AutoTolerance default, math notation). |
| src/common.jl | Adds docstring for equivalent-branch physical parameter extraction and LaTeX in outage math. |
| src/BranchesSeries.jl | Adds docstring describing series-chain reduced-branch representation. |
| src/BranchesParallel.jl | Adds docstrings for parallel-group reduced-branch representations (homogeneous/mixed). |
| src/BA_ABA_matrices.jl | Clarifies BA/ABA definitions and math notation in docstrings. |
| src/auto_tolerance.jl | Expands AutoTolerance docstring with scope rules and examples; LaTeX inequality. |
| src/apply_zero_impedance_reduction.jl | Pure formatting/indentation change for readability. |
| docs/src/tutorials/tutorial_VirtualPTDF_matrix.md | Removes legacy static tutorial (replaced by new Literate structure). |
| docs/src/tutorials/tutorial_VirtualLODF_matrix.md | Removes legacy static tutorial (replaced by new Literate structure). |
| docs/src/tutorials/tutorial_RadialReduction.md | Removes legacy static tutorial content (now covered via docstrings/how-tos/explanations). |
| docs/src/tutorials/tutorial_PTDF_matrix.md | Removes legacy static tutorial (superseded by new “Getting Started” + reference hubs). |
| docs/src/tutorials/tutorial_LODF_matrix.md | Removes legacy static tutorial (superseded by new “Getting Started” + reference hubs). |
| docs/src/tutorials/tutorial_Incidence_BA_ABA_matrices.md | Removes legacy static tutorial (covered by new reference and how-to content). |
| docs/src/tutorials/tutorial_DFAX.md | Removes legacy static tutorial (replaced by a new DFAX how-to). |
| docs/src/tutorials/tutorial_DegreeTwoReduction.md | Removes legacy static tutorial content (now covered via docstrings/explanations). |
| docs/src/tutorials/getting_started.md | Removes legacy markdown tutorial (replaced by Literate source). |
| docs/src/tutorials/getting_started.jl | Adds new Literate “Getting Started” end-to-end walkthrough. |
| docs/src/tutorials/analysis_at_scale.jl | Adds new Literate tutorial for large-scale screening using virtual matrices/caches. |
| docs/src/reference/public.md | Rebuilds the public reference page into structured autodocs sections. |
| docs/src/reference/network_matrices_overview.md | Replaces prior overview with a detailed indexing/taxonomy hub for all matrix types. |
| docs/src/reference/matrix_types.md | Adds a matrix type reference page (constructors, semantics, aliases). |
| docs/src/network_matrices.jl | Removes legacy overview page (replaced by new reference hub). |
| docs/src/index.md | Updates landing page copy + navigation pointers to new docs structure. |
| docs/src/how_to_guides/reproduce_dfax_values.jl | Adds Literate how-to reproducing industry DFAX values with executable examples. |
| docs/src/how_to_guides/diagnose_connectivity.jl | Adds Literate how-to for connectivity/islanding diagnosis. |
| docs/src/how_to_guides/contingencies.jl | Adds Literate how-to for defining/applying contingencies via VirtualMODF. |
| docs/src/how_to_guides/compute_network_matrices.md | Removes legacy how-to (superseded by new guides/reference). |
| docs/src/how_to_guides/choose_linear_solver.md | Removes legacy markdown how-to (replaced by Literate source). |
| docs/src/how_to_guides/choose_linear_solver.jl | Adds Literate how-to describing solver backends and platform defaults. |
| docs/src/how_to_guides/build_multiple_matrices.jl | Adds Literate how-to for reusing Ybus/A/BA intermediates efficiently. |
| docs/src/explanation/slack_conventions.md | Adds explanation page on slack/reference conventions and dist_slack implications. |
| docs/src/explanation/network_reduction_theory.md | Rewrites/expands reduction theory explanation with clearer math and structure. |
| docs/src/explanation/flowgate_methodology.md | Updates flowgate methodology page and cross-links to new DFAX how-to. |
| docs/src/explanation/equivalent_branches.md | Adds explanation of impedance vs rating aggregation for reduced branches. |
| docs/src/explanation/dc_power_flow_approximation.md | Expands and formalizes the DC approximation explanation + references. |
| docs/src/explanation/concurrency.md | Adds explanation of virtual-matrix concurrency behavior and KLU locking. |
| docs/src/explanation/computational_considerations.md | Refactors/condenses computational notes; points to AutoTolerance as authority. |
| docs/make.jl | Updates docs build to generate Literate content for tutorials and how-to guides. |
| docs/make_tutorials.jl | Generalizes tutorial generation to make_literate_folder for multiple subdirs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Performance ResultsPrecompile Time
Execution TimeCells show median (min–max) over 5 samples; delta compares medians.
|
kdayday
reviewed
Jul 23, 2026
kdayday
requested changes
Jul 23, 2026
kdayday
left a comment
kdayday
left a comment
Contributor
There was a problem hiding this comment.
Copilot comments mostly look to be legitimate format issues. I haven't double-checked its logic about some of the matrix math.
- Docs are failing on
AppleAccelerateLU-- I'd suggest just removing actual code examples to that. Please ask for re-review after they compile, I haven't looked at tutorials/how-tos - Elsewhere, we typically do 'using PowerNetworkMatrices' etc. in code examples, and then just e.g.,
Ybus(sys)instead ofPNM.Ybus(sys). Suggest simplifying - AI rewrote a bunch of math and code notation in docstrings, worth a visual review on whether that's being rendered/interpreted as intended
- I'm fine with the extension of literate logic to the how-to's in theory, but haven't looked at whether the content makes that worthwhile. Don't necessarily want to overcomplicate things if its not needed
m-bossart
reviewed
Jul 24, 2026
Contributor
|
I looked at the docs build but I second Kate's recommendation to look at the docstrings to make sure the math is rendered correctly. I think this is a good upgrade to the documentation, thanks @acostarelli |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 60 out of 60 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (3)
src/lodf_calculations.jl:350
- Same issue as above: the displayed LODF equation includes a trailing comma inside the math block.
src/ptdf_calculations.jl:420 - This math block equation line ends with a comma, which will render as part of the equation. Likely unintentional punctuation inside the formula.
src/ptdf_calculations.jl:477 - This math block equation line ends with a comma, which will render as part of the equation. Likely unintentional punctuation inside the formula.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 60 out of 60 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/lodf_calculations.jl:249
- When
tolis anAutoTolerance(the default),_lodf_from_systemcurrently ignores the caller’slinear_solverchoice entirely and always routes through the KLU-onlyLODF(A, ABA, BA)path. If a user passeslinear_solver != "KLU", this becomes a silent behavior change. Consider either validating and erroring with a clear message, or honoring the requested solver when possible (and documenting any restrictions).
kdayday
reviewed
Jul 28, 2026
kdayday
reviewed
Jul 28, 2026
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
kdayday
requested changes
Aug 18, 2026
kdayday
left a comment
kdayday
left a comment
Contributor
There was a problem hiding this comment.
I really like the new tutorials -- I definitely learned some things. Skimmed explanation, it looks good. 2 reference pages need a rework -- too complicated and overlapping with content in explanation and docstrings. Formatting comments included in tutorials and how-to's
Key concern to resolve: Undocumented internal functions with no docstrings that are used repeatedly in examples and should probably be both documented and exported
| # A [`PTDF`](@ref) (Power Transfer Distribution Factor) matrix is indexed by an **arc tuple** `(from_bus, to_bus)` and a **bus number**, answering what proportion of power injected at a given bus flows through a given branch (when withdrawn at the reference bus). One column of the [`PTDF`](@ref) corresponds to one bus's influence on every branch. | ||
|
|
||
| ptdf = PTDF(sys) | ||
| ptdf[(103, 104), 103] |
Contributor
There was a problem hiding this comment.
Maybe separate these two lines, so we can also see the printed output of the PTDF call first, then move the "One column of the PTDF corresponds to one bus's influence on every branch:" lead-in before the second line?
| # We can filter for monitored arcs that absorb flow past a given threshold. | ||
|
|
||
| outaged = (103, 104) | ||
| monitored = [(arc, lodf[arc, outaged]) for arc in lodf.axes[1] if arc != outaged] |
Contributor
There was a problem hiding this comment.
I'd break up this code block into a couple blocks, repeating the indexing meanings again for lines 1 and 2
| sort!(monitored; by = pair -> -abs(pair[2])) | ||
| monitored | ||
|
|
||
| # Branch `(102, 103)` inherits the entire flow of the outaged line, a factor of `-1.0`, because it is the series partner on the far side of bus `103`. The two parallel paths `101–115–102` and `101–117–118–104` each absorb roughly `65%`, and branch `(102, 104)` takes the remaining `35%`. Every other branch is unaffected. |
Contributor
There was a problem hiding this comment.
This analysis is helpful. It presupposes that you know a bit about the topology -- is it possible to introduce the parallel paths, etc. (or show how to figure that out with Sienna?) briefly here or up in the introduction? Ignore if it would take too long or detour too much
|
|
||
| # A negative factor means the redistributed flow runs *against* the monitored branch's `(from, to)` orientation. Individual factors can also be read directly. | ||
|
|
||
| lodf[(102, 103), (103, 104)] |
Contributor
There was a problem hiding this comment.
I'd reiterate again the indexing in comment or code comment
|
|
||
| # ## Step 1 — Load the network | ||
|
|
||
| # Network matrices are built from a [`PowerSystems.System`](@extref PowerSystems.System). |
Contributor
There was a problem hiding this comment.
Suggested change
| # Network matrices are built from a [`PowerSystems.System`](@extref PowerSystems.System). | |
| # Network matrices are built from a [`PowerSystems.System`](@extref PowerSystems.System). Load an example test system using [`PowerSystemCaseBuilder.build_system`](@extref): |
| signatures when you need to fix a concrete storage layout. All are exported, as is | ||
| `YBUS_ELTYPE`. | ||
|
|
||
| | Alias | Underlying type | Meaning | |
Contributor
There was a problem hiding this comment.
Can't find docstrings for these types. Need to be added? And this table integrated with Matrix Taxonomy table?
| (`DC_PTDF_Matrix`, `DC_ABA_Matrix_Factorized`, `AC_Ybus_Matrix`, …) are on the | ||
| [Matrix type reference](matrix_types.md). | ||
|
|
||
| ## Constructing matrices |
Contributor
There was a problem hiding this comment.
Check if context covered by tutorials/how-to's and remove?
| BA) are computed once and reused. See | ||
| [How to Build Multiple Matrices Without Repeating Work](@ref). | ||
|
|
||
| ## Arc-tuple indexing |
Contributor
There was a problem hiding this comment.
There is so much content from here on down, it's losing me.
| - **Exported:** [`get_ptdf_data`](@ref), [`get_lodf_data`](@ref), | ||
| [`get_partial_lodf_row`](@ref), [`get_network_reduction_data`](@ref), | ||
| [`get_system_uuid`](@ref). | ||
| - **Internal (call via `PNM.`):** `get_data`, `get_axes`, `get_lookup`, |
Contributor
There was a problem hiding this comment.
@m-bossart Concerned these should just be exported. I saw some of these in tutorials, too. Can't even find docstrings in internal API.
|
|
||
| The accepted element types for `row` and `column`, and how each resolves: | ||
|
|
||
| | Index value | Resolves via | Supported | |
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.
No description provided.