Jb tutorial edits#357
Open
jbrossman wants to merge 17 commits into
Open
Conversation
Build out uc-power-flow-in-the-loop.md significantly more and add simple installation instructions to the top of the other two tutorials.
Now compatible with PowerNetworkMatrices v0.20.0
Compiled tutorials with fixed version inconsistencies
Contributor
Performance ResultsPrecompile Time
Solve Time
|
Update toml to remove unnecessary package that was causing issues in the draft PT
… models vs. solvers (#371) * simplify hessian matrix structure initialization * add test of RH w/ headroom slack * address copilot's comments Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * simplify HomotopyHessian constructor; switch to IS.@assert_op Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * restore defensive fill-with-ones in HomotopyHessian constructor Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Update org/domain/workspace references from NREL-Sienna to Sienna-Platform (#358) * Initial plan * docs: update org, docs host, and slack workspace references Agent-Logs-Url: https://github.com/Sienna-Platform/PowerFlows.jl/sessions/6568f54e-0a28-4f56-b017-353e8b212eb1 Co-authored-by: jd-lara <16385323+jd-lara@users.noreply.github.com> * docs: fix README typo in updated contribution line Agent-Logs-Url: https://github.com/Sienna-Platform/PowerFlows.jl/sessions/6568f54e-0a28-4f56-b017-353e8b212eb1 Co-authored-by: jd-lara <16385323+jd-lara@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jd-lara <16385323+jd-lara@users.noreply.github.com> * reject LCC systems in RobustHomotopyPowerFlow LCCs add state variables to the Jacobian that the homotopy Hessian formulation does not account for, producing a dimension mismatch deep inside the solve. Fail fast at HomotopyHessian construction with a clear error message. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * add storage to possibly be PV (#359) * fix: update fields for tap transformers and v35 exporter default values (#360) * Bump version from 0.16.3 to 0.16.4 * Add literate.jl docs infrastructure (#356) * Add precommit format yaml * Add docs deps * Add docs preview cleanup * Add make_tutorials infrastructure * add generated files to gitignore * Translate tutorial to literate and fix type name and DC reporting bugs * Add prettyurls * Tidy dc power flow docstrings * Move LCC to explanation * Remove double execution * Compute url for tagged versions * Filter tutorial files with endswith * Add links as absolute url * Single image insert * Use non-greedy capture * Switch to abspath * Add input/output documentation * Add kwargs to base.show override * Clarify image regex * correct last org refs * Add local vs github link logic * move code for reuse * udpate LCC indexing * add rectangular residual calculator * add rectangular jacobian * export new type * add NR power flow for rectangular * add the setup code * use the new klu branch * exports and definitions * AI assisted testing * add code to catch unsopported methods * add the testing and docstring * bump psy version * apply the fixes from copilot's comment * delete plans * fix copilot suggestion * address comments * update static arrays * apply memory reduction suggestions * fix format and docs * test allocation regressions * delete plan * fix jacobians * add more testing * address PR comments and testing * fix the ZIP discrepancy * last LCC updates * Fix LCC Jacobian: correct dQ/dV, dQ/dt; add missing inverter-side entries The simplified-φ Q-derivatives in lcc_utils.jl had algebra errors (spurious V factor on the first term, sin²(φ) instead of V·sin(φ) in the second, missing sign(I_dc)). The Jacobian assembly was also missing all six bus-side entries on the inverter terminal — only the rectifier mirror was wired up — so finite-difference verification on any LCC system fails by construction. Add the missing entries (and their sparsity pattern), guard the helpers against the sin(φ)→0 clamp boundary (true derivative is 0 there), and log a warning when the arccos argument leaves [-1,1]. Re-enables verify_jacobian on the LCC test system, which was previously commented out for exactly these reasons. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Address Copilot review: clean warning string, clarify test comment Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix LCC docs inconsistency; global const for sin \phi tol * Deduplicate LCC tail residual/Jacobian; asymptotic Jacobian verification Factor the LCC tail residual rows and the LCC Jacobian tail-block scalar coefficients into shared helpers in lcc_utils.jl, consumed by both the polar and rectangular CI formulations. Mirrors the dispatch pattern of _update_ybus_lcc!. Drops the implicit allocations from polar's broadcast slicing of the LCC tail and prevents the two formulations from drifting on future LCC fixes. Stale docstring in the rect Jacobian about the inverter-side bus diagonal block (corrected upstream in be9ac82) is removed and replaced with a forward-looking note about porting the true-φ Q-derivative helpers. Replace the fixed-tolerance FD-parity Jacobian tests (rect LCC, four rect Jacobian testsets, polar verify_jacobian) with an asymptotic verifier in test/test_utils/jacobian_verification.jl that checks the linearization remainder decays as O(Δx²) under a geometric Δx-sweep (matching the methodology already used by test_homotopy_hessian.jl). Filters Δx-points that fall to the FP-noise floor and, on failure, prints per-Δx remainders, observed Taylor orders, and a worst-row symbolic-vs-FD diff for the suspect Jacobian entry. Each verification point now applies a deterministic random perturbation so we no longer evaluate at flat-start (θ=0) or polar-converged states where sin-style cross-terms silently vanish. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Polar LCC P-row Jacobian: migrate to true-φ via dP/dV, dP/dt helpers Polar's bus-P-row entries (∂P/∂V_fb, ∂P/∂t_fb, and the matching inverter-side entries) and the tail-row × bus-column entries (∂F_t/∂V) were using the α-approximation — treating φ as a constant equal to α and dropping the ∂φ/∂(V, t) chain terms. That's exact only when the LCC commutation reactance x_t is zero; for x_t > 0 it leaves out a real term. Add _calculate_dP_dV_lcc and _calculate_dP_dt_lcc parallel to the existing _calculate_dQ_d{V,t,α}_lcc helpers. Unlike the Q helpers, neither requires a sin(φ)→0 guard: differentiating cos(φ) introduces a -sin(φ) factor that cancels the 1/sin(φ) from ∂φ/∂x, leaving a regular closed form. ∂P/∂α is left on the α-approximation since the analogous cancellation makes that formula already exact. No observable behavior change in the current test suite — every LCC test fixture (case5_2_lcc.raw, the synthetic systems in test_jacobian.jl) has x_t = 0, which collapses the new term to zero. The change matters for the first system we hit with nonzero commutation reactance. Polar's tail × tail block (d_Ft_* fields of _lcc_jacobian_scalars) is still on the α-approximation: it's shared with rect via the NamedTuple, and migrating it would either touch the rect path or require splitting the helper. Left for a follow-up. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * True-φ LCC Jacobian: polar P-row clamp guard + rect migration The polar P-row entries had a latent clamp boundary bug: the α-approximation form (cos(α), sin(α)) is *algebraically equivalent* to true-φ in the interior — the ϕ-definition identity cos(α) - cos(ϕ) = x_t·I_dc/(√2·V·tap) makes the chain correction collapse into the α-trig form — but the identity *breaks at the clamp*, where ϕ is pinned and ∂ϕ/∂x = 0. The α-approximation form keeps using cos(α), sin(α) regardless, mismatching the residual (which sees cos(ϕ_clamp), sin(ϕ_clamp) = 0). Add _calculate_dP_d{V,t,α}_lcc helpers parallel to the existing _calculate_dQ_d* set; each guards sin(ϕ) → 0 so the chain term is dropped at the clamp (where ∂ϕ/∂x = 0 physically). The dP/dV and dP/dt helpers happen to be regular at the clamp (no 1/sin(ϕ)), but the guard still matters because the α-approximation form they replace uses cos(α), not cos(ϕ_clamp). The dP/dα helper has the same 1/sin(ϕ) singularity the Q helpers do. Rewire _lcc_jacobian_scalars to call these helpers for the d_Ft_* tail × tail entries — a correctness fix for both polar and rect tail × tail (the tail equations and tail columns are formulation-invariant). New test in test_jacobian.jl constructs a 3-bus system with large inverter x_t and small extinction angle to force ϕ_i = π (clamp); without the migration the asymptotic verifier reports observed Taylor order ≈ 1 on the inverter-α and tap_i columns. Then migrate rect's LCC Jacobian: rect's α-approximation is *not* algebraically equivalent to true-ϕ even in the interior (no analogous identity, because the substitution cos(α) → cos(ϕ) doesn't factor through (e, f) the way it does through V). Add _dphi_d{V,t,α}_lcc helpers exposing ∂ϕ/∂x with the same sin(ϕ) → 0 guard, and rewrite _set_entries_for_lcc_rect! to use the universal admittance form F_real = -A·u(ϕ), F_imag = -A·w(ϕ) (with the inverter sign convention absorbed into cos(ϕ_i), sin(ϕ_i)) plus the per-derivative chain terms. Two new rect tests cover nonzero xc in the interior and the inverter ϕ-clamp; both pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Replace stale α-approx comment in polar; expose dP_dt_* in scalars The "True-φ ∂P/∂V and ∂P/∂t for the rectifier side..." comment block in `_set_entries_for_lcc` was written in the first polar P-side migration commit and is no longer accurate: - "polar α-approximation drops the ∂φ/∂Vm chain term — which vanishes only when x_t = 0": false. The α-form is *algebraically equivalent* to true-ϕ in the interior via the ϕ-definition identity `cos(α) − cos(ϕ) = x_t·I_dc/(√2·V·tap)`; the chain term doesn't vanish, it cancels into the α-trig form. - "For ∂P/∂α the analogous cancellation makes the α-approximation exact, so those entries stay on `common_alpha_*`": also false now. The α-form is exact only in the interior; at the clamp it's wrong (the inverter-ϕ-clamp test caught it), so we migrated ∂P/∂α to the guarded `s.dP_dα_*` in the rect-migration commit. Replace with a concise comment pointing at the true-ϕ equivalence-in- the-interior and the clamp-guard rationale, and reference the test that exercises the clamp. While here: expose `dP_dt_fb`, `dP_dt_tb` from `_lcc_jacobian_scalars` alongside the existing `dP_dV_*` and `dP_dα_*`, and have polar read them all from the scalars NamedTuple instead of recomputing the same helper calls locally. No behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * rename * split abstraction between polar and rectangular * testing and performance changes * PR feedback: PV-bus Q-derivatives, noise-floor docs, cleanups Address Copilot review on PR #366: * **Bug: PV-bus LCC Q-derivatives missing** (`src/ac_power_flow_jacobian.jl`): ∂Q/∂tap and ∂Q/∂α at the LCC AC terminal were inside the `bus_type == PQ` block. For PV terminals the Q-mismatch row still depends on tap/α through Q_lcc, so those entries should be filled regardless of bus type — the V-dependent entries (∂{P,Q}/∂V and the tail-row × V chain rule) are the only ones that need PQ gating. Move bus-row × tail-column writes out of the PQ block; gate only the V-column writes. Add a "Jacobian verification with LCC at a PV terminal" test that exercises this regime. * **Noise-floor sensitivity audit** (`test/test_utils/jacobian_verification.jl`): document the analytical detection threshold for entry-level Jacobian errors: `|δ|_min ≈ c · eps · ||F0|| / Δx_max ≈ 2.2e-9 · max(||F0||,1)` with current defaults. Print the sensitivity floor in the failure diagnostic so a reader can tell what a passing test does *not* exclude. Tighten `Jv` type to `AbstractMatrix` (Copilot noted the failure path uses `Jv[row, j]` which requires `getindex`). * **Cleanups**: replace `sqrt(6)/π` with the existing `SQRT6_DIV_PI` global const in `lcc_utils.jl` (3 occurrences). Trim the long-winded comment block in polar `_set_entries_for_lcc` introduced earlier this PR. Replace stale "Both thyristor angles must be strictly positive" comment in the polar LCC verify_jacobian test with one that accurately points at the separate inverter-ϕ-clamp test for boundary coverage. Deferred to a follow-up commit: setpoint_at_rectifier=false branching (needs sparsity-structure additions on both polar and rect). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix warm start for rectangular * format change * PR comments * add more testing * wrap test in logging * Bump version from 0.16.4 to 0.17.0 * Accept narrowly-typed Dicts for ACPowerFlow solver_settings (#369) Broaden the kwarg type to AbstractDict and coerce internally to Dict{Symbol, Any}, so that callers can pass a plain literal like `Dict(:maxIterations => 50)` (inferred as Dict{Symbol, Int}) without having to spell out the value type. Dict is invariant in its parameters, so the prior `Dict{Symbol, Any}` annotation rejected these. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix broken test * LM performance improvements (#338) * LM performance improvements * copilot code review; fix x0 kwarg * add total iterations to RH method * add method comparison script * review the PR and add rectangular testing --------- Co-authored-by: Jose Daniel Lara <jdlara@berkeley.edu> * Bring branch up to date with main; restructure power-flow docs around models vs. solvers Agent-Logs-Url: https://github.com/Sienna-Platform/PowerFlows.jl/sessions/e631d3fd-287d-4d0a-92c1-1bfbfb3a6c01 Co-authored-by: jd-lara <16385323+jd-lara@users.noreply.github.com> --------- Co-authored-by: Luke Kiernan <lkiernan@nrel.gov> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: jd-lara <16385323+jd-lara@users.noreply.github.com> Co-authored-by: Rodrigo Henríquez-Auba <rodrigomha@gmail.com> Co-authored-by: Marck Llerena V. <140716266+mcllerena@users.noreply.github.com> Co-authored-by: kdayday <kdayday@users.noreply.github.com> Co-authored-by: Jose Daniel Lara <jdlara@berkeley.edu> Co-authored-by: Luke Kiernan <86331877+luke-kiernan@users.noreply.github.com> Co-authored-by: Copilot <copilot@github.com>
Member
|
@copilot resolve the merge conflicts in this pull request |
Agent-Logs-Url: https://github.com/Sienna-Platform/PowerFlows.jl/sessions/93105271-3d4d-439e-9528-b55eeffab40d Co-authored-by: jd-lara <16385323+jd-lara@users.noreply.github.com>
Contributor
Resolved and pushed in commit abfb75b. I merged Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Co-authored-by: jd-lara <16385323+jd-lara@users.noreply.github.com>
Member
|
@copilot the documentation build is failing, fix it to be able to see the rendering of the preview |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR rewrites the PowerFlows.jl tutorial set to address Sienna-Platform/Sienna#65: it replaces the single Literate-generated "Solving a Power Flow" tutorial with three hand-written Markdown tutorials (a rewritten basic walkthrough, a multi-period DC validation of a UC dispatch, and a power-flow-in-the-loop UC simulation), adds a new conceptual explanation page, restructures the docs build to include them, and adds a small convenience overload to solve_power_flow for DC methods so the new tutorials can call it without an explicit FlowReporting argument.
Changes:
- Add three new tutorial pages and one explanation page; reorganize
docs/make.jlanddocs/Project.toml(including a[sources]pin of PowerSimulations tomain) to build them. - Add a 2-argument
solve_power_flow(::AbstractDCPowerFlow, ::System)overload defaulting toFlowReporting.ARC_FLOWS. - Add two scratch scripts under
scripts/and dropgradient_descent_ac_power_flow.jlfrom the Public API autodocs page list.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| src/solve_dc_power_flow.jl | New 2-arg convenience overload defaulting FlowReporting to ARC_FLOWS. |
| docs/make.jl | Loads PowerSimulations/HiGHS, swaps tutorial pages, adds an env-gated warnonly. |
| docs/Project.toml | Adds HiGHS + PowerSimulations deps and pins PowerSimulations to upstream main. |
| docs/src/reference/api/public.md | Renames the page title and removes the gradient-descent module from autodocs. |
| docs/src/explanation/models-and-solvers.md | New conceptual page on evaluation models vs. AC solver algorithms. |
| docs/src/tutorials/solving-a-power-flow.md | Hand-written replacement for the old Literate tutorial, expanded to cover AC formulations and solver tags. |
| docs/src/tutorials/power-flow-after-unit-commitment.md | New tutorial using multi-period DC PF to validate a synthetic UC dispatch. |
| docs/src/tutorials/uc-power-flow-in-the-loop.md | New tutorial wiring an AC power flow into a PowerSimulations UC simulation. |
| scripts/setup_test_env.jl | Ad-hoc Pkg.develop + Pkg.instantiate helper. |
| scripts/check_columns.jl | Ad-hoc DC flow-result inspection script. |
Comments suppressed due to low confidence (18)
docs/Project.toml:17
- Pinning
PowerSimulationsto themainbranch of an upstream repo via[sources]makes every docs build non-reproducible and means an unrelated breaking change upstream can break this repository's docs CI without any change here. Prefer a registered version with a[compat]entry (e.g.PowerSimulations = "^X.Y"). If amain-only feature is genuinely required, pin to a specific commitrev = "<sha>"so the build is at least reproducible, and add a TODO to switch back to a tagged release.
[sources]
PowerSimulations = {url = "https://github.com/NREL-Sienna/PowerSimulations.jl", rev = "main"}
docs/src/tutorials/uc-power-flow-in-the-loop.md:50
-
disable_logging(Logging.Error)disables all log messages at theErrorlevel and below, which means it silencesError(andWarn,Info,Debug) too. Ifsolve_power_flowfails to converge in this tutorial it logs atErrorlevel (seesolve_power_flowreturningmissing"and logs an error", which is documented in the sibling tutorial); with this setting the user will see nothing. The other tutorials in this PR (solving-a-power-flow.md,power-flow-after-unit-commitment.md) usedisable_logging(Logging.Warn). Consider matching them.
using Logging
disable_logging(Logging.Error)
docs/src/reference/api/public.md:23
- The Pages list dropped
gradient_descent_ac_power_flow.jl, butGradientDescentACPowerFlowis still publicly exported fromsrc/PowerFlows.jl(line 13) and the include is still present (src/PowerFlows.jl:75). After this change, its docstring will no longer appear in the Public API page, leaving an exported symbol undocumented in the rendered docs. Either re-add the file to the Pages list, un-export the symbol, or remove it entirely from the source.
```@autodocs
Modules = [PowerFlows]
Public = true
Private = false
Pages = [
"power_flow_types.jl",
]
**docs/make.jl:43**
* `warnonly = true` (via env var) silently turns Documenter errors into warnings for the entire build. This is fine as an emergency escape hatch, but if `POWERFLOWS_DOCS_WARNONLY=true` is set in CI it will mask real broken cross-references introduced by these new tutorials (e.g. `@ref "Installation"`, `@ref "Public API"`, the `[`solve_and_store_power_flow!`](@ref)` references, etc., none of which appear to be defined anywhere I can find). Recommend keeping it `false` in CI and gating on local debugging only.
warnonly = get(ENV, "POWERFLOWS_DOCS_WARNONLY", "false") == "true",
**docs/src/tutorials/power-flow-after-unit-commitment.md:133**
* Cross-reference uses dotted enum syntax inside a Documenter `@ref` link: `[`FlowReporting`](@ref).BRANCH_FLOWS`. Documenter only links the `FlowReporting` text and renders `.BRANCH_FLOWS` as plain markdown that immediately follows the link, which is awkward and easy to misread as part of the link text. The same pattern is used several times in this file (lines 94, 133). Consider writing it as plain code: `` `FlowReporting.BRANCH_FLOWS` `` (matching how the source docstrings in `src/solve_dc_power_flow.jl:180` express it), or as two separate references.
Solve all 24 time steps in a single call. Using FlowReporting.BRANCH_FLOWS reports
one row per physical branch by its PowerSystems.jl name, which lets us look up ratings directly
from the System:
results = solve_power_flow(data, sys, FlowReporting.BRANCH_FLOWS)
results is a Dict{String, Dict{String, DataFrame}}. The outer key is the time step
name ("1" through "24"). The inner dictionary has keys "bus_results",
"flow_results", and "lcc_results".
Inspecting Results
Look at bus results for hour 1:
results["1"]["bus_results"]
Notice that Vm (voltage magnitude) is 1.0 for all buses and Q_gen, Q_load are 0.
This is expected for DC power flow, which assumes flat voltage profiles and ignores
reactive power entirely.
Look at line flows for hour 1:
results["1"]["flow_results"]
Notice that P_from_to and P_to_from are not equal and opposite, and P_losses
is non-zero for resistive branches. Although DC power flow ignores reactive power and
assumes flat voltage magnitudes, PowerFlows.jl computes a first-order active power loss
estimate as
(zero resistance) will still show zero losses.
Checking for Overloads
Build a branch rating lookup from the System — flow_name in the results matches
the PowerSystems.jl branch name when using FlowReporting.BRANCH_FLOWS reporting:
**src/solve_dc_power_flow.jl:214**
* This new convenience overload duplicates the dispatch pattern already used for AC power flow (`src/solve_ac_power_flow.jl:79-85`), but it intentionally omits `; kwargs...`. If any caller passes keyword arguments to the 2-argument DC form (consistent with the AC overload that forwards `kwargs`), they will hit a `MethodError`. Consider matching the AC signature by accepting and forwarding `kwargs...` for symmetry, even if no current keywords are consumed downstream.
function solve_power_flow(
pf::T,
sys::PSY.System,
) where {T <: AbstractDCPowerFlow}
return solve_power_flow(pf, sys, FlowReporting.ARC_FLOWS)
end
**docs/src/tutorials/uc-power-flow-in-the-loop.md:106**
* `use_slacks = true` is described here as a way to keep the simulation feasible "when there is a small mismatch between the PTDF-based UC network model and the full AC power flow". That's not what `use_slacks` does in the PTDFPowerModel network formulation in PowerSimulations — it adds bus power-balance slack variables to the UC formulation itself, so the *UC optimization* can violate nodal balance. It has nothing to do with reconciling the post-hoc AC power flow output, which is computed after the UC solves and writes results back into `sys` without feedback into the optimizer. Please reword to avoid misleading users about the slack semantics.
use_slacks = true allows the simulation to remain feasible when there is a small
mismatch between the PTDF-based UC network model and the full AC power flow. Set
use_slacks = false in production workflows where you want the simulation to fail loudly
if the AC power flow cannot match the committed dispatch.
**docs/src/tutorials/uc-power-flow-in-the-loop.md:201**
* Similarly, the prose around lines 198-201 says "At each interval, the AC power flow then re-evaluated these setpoints on the full network — any mismatch was absorbed by the slack variables configured via `use_slacks = true`." This is again incorrect — UC slack variables don't absorb post-solve AC mismatches; AC mismatch shows up at the slack bus / as failure to converge / as the AC solver's own results. Please correct.
will show zero output. The dispatch levels here are the UC solution from the PTDF-based
network model. At each interval, the AC power flow then re-evaluated these setpoints on
the full network — any mismatch was absorbed by the slack variables configured via
use_slacks = true.
**docs/src/tutorials/solving-a-power-flow.md:121**
* The tutorial states `ACPowerFlow` is the alias and `ACPolarPowerFlow` is the underlying type ("`ACPowerFlow` is an alias for `ACPolarPowerFlow`"). Worth verifying against `src/power_flow_types.jl` that the alias direction is exactly this — in some Julia codebases it's defined the other way (typedef vs `const`). If `ACPolarPowerFlow` is actually the alias for `ACPowerFlow`, this sentence is backwards.
ACPowerFlow is an alias for ACPolarPowerFlow. It solves the
full non-linear AC equations using a per-bus state (Vᵢ, θᵢ) and a real
power-mismatch residual:
**docs/src/tutorials/solving-a-power-flow.md:130**
* The note says "Sienna does not yet support multi-period AC power flows". This contradicts the multi-period AC support that exists in PowerFlows.jl (e.g. `ACPowerFlow` constructors accept `time_steps` and there are multi-period AC code paths in `src/solve_ac_power_flow.jl`). Please verify the current state and either reword (e.g. "the single-period convenience overload returns a flat Dict; multi-period AC is available via `PowerFlowData`") or remove the claim if outdated.
AC results are returned as a flat Dict{String, DataFrame} with the same keys
as before. (Sienna does not yet support multi-period AC power flows, so there is
no outer time-step layer.)
**docs/src/tutorials/solving-a-power-flow.md:98**
* "Purely inductive branches will show zero losses." A purely inductive branch has zero resistance (R = 0), so the first-order `R · P²` loss estimate is zero — that's correct. But the prior sentence says `P_losses` follows `R · P²`, and the loss for `R = 0` branches is "zero" regardless of inductance, not specifically because of inductance. Saying "purely reactive" or "lossless (R = 0)" branches would be more precise.
Q_from_to and Q_to_from are zero, and P_losses follows a first-order
R · P² estimate. Purely inductive branches will show zero losses.
**docs/src/tutorials/power-flow-after-unit-commitment.md:128**
* Same issue here as in `solving-a-power-flow.md`: "Purely inductive branches (zero resistance) will still show zero losses." Purely inductive is not synonymous with zero resistance — please say "lossless (R = 0)" branches or similar.
estimate as
(zero resistance) will still show zero losses.
**docs/src/tutorials/power-flow-after-unit-commitment.md:159**
* The tutorial iterates `for (t, step_results) in results` over a Dict and then `push!`es rows onto `overloads` keyed by `t`. Dict iteration order is undefined in Julia, so the resulting `overloads` DataFrame can have rows in arbitrary order across builds — making the rendered docs output non-deterministic. Sort by integer time step (`for t in sort(collect(keys(results)); by = x -> parse(Int, x))`) for reproducible docs output and a friendlier reader experience.
for (t, step_results) in results
for row in eachrow(step_results["flow_results"])
rating = get(ratings, row.flow_name, Inf)
if abs(row.P_from_to) > rating
push!(overloads, (t, row.flow_name, row.P_from_to, rating))
end
end
end
**docs/src/tutorials/power-flow-after-unit-commitment.md:140**
* Setting `ratings = Dict(name => rating * base_power)` here treats `get_rating(b)` as a per-unit quantity that must be multiplied by `get_base_power(sys)` to get MW. Depending on the current `UnitSystem` in `sys` (NATURAL vs. SYSTEM_BASE vs. DEVICE_BASE), `get_rating` may already return a value in MW, in which case this overstates ratings by a factor of `base_power` and the overload check will (silently) never trigger. Recommend wrapping the dict comprehension in `PowerSystems.with_units_base(sys, PSY.UnitSystem.NATURAL_UNITS) do ... end`, or using `get_rating(b)` consistently in SYSTEM_BASE per-unit and comparing per-unit against `row.P_from_to / base_power`.
ratings = Dict(
PowerSystems.get_name(b) => PowerSystems.get_rating(b) * PowerSystems.get_base_power(sys)
for b in PowerSystems.get_components(PowerSystems.ACBranch, sys)
)
**docs/src/tutorials/power-flow-after-unit-commitment.md:89**
* The loop only modifies `bus_active_power_withdrawals` per time step but reassigns `bus_active_power_injections` to the unchanged `base_injections` every iteration. The first assignment is a no-op (since `data.bus_active_power_injections[:, t]` already equals `base_injections` from the constructor for `t=1`, and is overwritten to the same value for every `t`). If the intent is "loads scale, generation stays constant", that's what happens — but consider dropping the redundant injection write and adding a comment that this models load-only variation, since otherwise readers may believe the loop is also varying injections.
for t in 1:24
load_scale = 0.8 + 0.2 * sin(2π * (t - 1) / 24)
data.bus_active_power_injections[:, t] .= base_injections
data.bus_active_power_withdrawals[:, t] .= base_withdrawals .* load_scale
end
**docs/src/tutorials/power-flow-after-unit-commitment.md:83**
* The tutorial reads `data.bus_active_power_injections[:, 1]` and `data.bus_active_power_withdrawals[:, 1]` as the "base" values immediately after constructing `PowerFlowData`. Verify that the freshly constructed multi-period `PowerFlowData` actually populates *all* time steps with the system snapshot (and not e.g. column 1 only with zeros elsewhere). If the constructor leaves later columns at zero, then `base_injections` from column 1 is fine but readers building real workflows on top of this will be surprised. A short note clarifying the initial population semantics would help.
base_injections = copy(data.bus_active_power_injections[:, 1])
base_withdrawals = copy(data.bus_active_power_withdrawals[:, 1])
**docs/src/tutorials/uc-power-flow-in-the-loop.md:141**
* The tutorial reports that the 5-bus Matpower DA system "has 6 periods of time series data, so you will see 6 rows" and `steps = 6` is used in the `Simulation` constructor. This couples the simulation length to the time-series data length without explaining why; a reader copy-pasting this onto a system with different time-series resolution will get cryptic errors. Briefly note that `steps` must be ≤ the available horizon, and explain where the 6 comes from.
sim = Simulation(;
name = "uc_with_pf",
steps = 6,
models = models,
sequence = sequence,
simulation_folder = joinpath("outputs", "simulation"),
)
**docs/src/tutorials/uc-power-flow-in-the-loop.md:141**
* `mkpath(joinpath("outputs", "psse"))` is called here, but the `PSSEExportPowerFlow` was already configured with the same `export_dir` at line 69 and `overwrite = true`. Most exporters create the directory themselves; the `mkpath` may be redundant. Either drop it, or add a brief comment explaining why both are needed. Also, writing into a relative `"outputs/"` directory pollutes the user's current working directory when they run the tutorial — consider `mktempdir()` or `joinpath(@__DIR__, "outputs")` for cleaner behavior.
mkpath(joinpath("outputs", "simulation"))
mkpath(joinpath("outputs", "psse"))
sim = Simulation(;
name = "uc_with_pf",
steps = 6,
models = models,
sequence = sequence,
simulation_folder = joinpath("outputs", "simulation"),
)
</details>
---
💡 <a href="/Sienna-Platform/PowerFlows.jl/new/main?filename=.github/instructions/*.instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Add Copilot custom instructions</a> for smarter, more guided reviews. <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Learn how to get started</a>.
| @@ -0,0 +1,3 @@ | |||
| import Pkg | |||
| Pkg.develop(; path = ".") | |||
Comment on lines
+1
to
+4
| using PowerFlows, PowerSystemCaseBuilder | ||
| sys = build_system(MatpowerTestSystems, "matpower_case5_sys") | ||
| dc = solve_power_flow(DCPowerFlow(), sys) | ||
| println(names(dc["1"]["flow_results"])) |
| PowerSimulations = "e690365d-45e2-57bb-ac84-44ba829e73c4" | ||
| PowerSystemCaseBuilder = "f00506e0-b84f-492a-93c2-c0a9afc4364e" | ||
| PowerSystems = "bcd98974-b02a-5e2f-9ee0-a103f5c450dd" | ||
| PrettyTables = "08abe8d2-0d0c-5749-adfa-8a2ac140af0d" |
| "How-to-Guides" => "how-tos/stub.md", | ||
| "Tutorials" => Any[ | ||
| "Solving a Power Flow" => "tutorials/generated_solving_a_power_flow.md", | ||
| "Solving a Power Flow" => "tutorials/solving-a-power-flow.md", |
Comment on lines
+3
to
+18
| To get started, ensure you have followed the [installation instructions](@ref "Installation"). Start Julia from the command line if you haven't already. | ||
|
|
||
| In this tutorial, you'll configure a unit commitment (UC) simulation in | ||
| [`PowerSimulations.jl`](https://nrel-sienna.github.io/PowerSimulations.jl/stable/) that calls an AC power flow solver at every dispatch interval | ||
| automatically. By the end, you'll have a simulation where the committed dispatch is | ||
| validated against the full AC network model at each hour, with results written back into | ||
| the system and optionally exported to PSS/e format. | ||
|
|
||
| !!! note | ||
| This tutorial requires [`PowerSystemCaseBuilder.jl`](https://nrel-sienna.github.io/PowerSystemCaseBuilder.jl/stable/), | ||
| [`PowerSimulations.jl`](https://nrel-sienna.github.io/PowerSimulations.jl/stable/), and | ||
| [`PowerSystems.jl`](https://nrel-sienna.github.io/PowerSystems.jl/stable/) in addition to `PowerFlows.jl`. Make sure | ||
| all are installed in your Julia environment before proceeding. | ||
|
|
||
| If you want to run power flow independently *after* a UC simulation rather than during | ||
| it, see the tutorial [Validating a UC Dispatch with Multi-Period Power Flow](@ref). |
| For a fuller treatment of that distinction, see the | ||
| [Evaluation Models vs. Solver Algorithms](@ref) explanation page. | ||
|
|
||
| To get started, ensure you have followed the [installation instructions](@ref "Installation"). |
| @@ -0,0 +1,185 @@ | |||
| # Validating a UC Dispatch with Multi-Period Power Flow | |||
|
|
|||
| To get started, ensure you have followed the [installation instructions](@ref "Installation"). Start Julia from the command line if you haven't already. | |||
Comment on lines
+209
to
+214
| function solve_power_flow( | ||
| pf::T, | ||
| sys::PSY.System, | ||
| ) where {T <: AbstractDCPowerFlow} | ||
| return solve_power_flow(pf, sys, FlowReporting.ARC_FLOWS) | ||
| end |
| @@ -1,4 +1,4 @@ | |||
| # PowerFlows | |||
| # Public API | |||
…exclude PS tutorials Agent-Logs-Url: https://github.com/Sienna-Platform/PowerFlows.jl/sessions/212e4ba2-ba53-4199-959f-a29ae550ac0e Co-authored-by: jd-lara <16385323+jd-lara@users.noreply.github.com>
Contributor
Pushed a fix in 581bd34. Root cause: Changes:
The new Documentation workflow run is currently Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Agent-Logs-Url: https://github.com/Sienna-Platform/PowerFlows.jl/sessions/bcb3f0f5-76de-4bac-8ab9-fc1949ef5010 Co-authored-by: kdayday <12451220+kdayday@users.noreply.github.com>
Member
|
@copilot merge main into this branch and update any conflicts. Don't stop until you make the documentation run and deploy successfully |
# Conflicts: # docs/make.jl Co-authored-by: jd-lara <16385323+jd-lara@users.noreply.github.com>
jd-lara
reviewed
May 19, 2026
Member
There was a problem hiding this comment.
@kdayday we should delete this and. this should go in PSI or POM
Agent-Logs-Url: https://github.com/Sienna-Platform/PowerFlows.jl/sessions/41700a84-5b9a-441c-aa21-785c1b71dcef Co-authored-by: jd-lara <16385323+jd-lara@users.noreply.github.com>
Contributor
Merged The previous docs build failed with The current Documentation workflow run for f330a8c is in Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
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.
Tutorial edits to address issue Sienna-Platform/Sienna#65