Handle negative net demand in DC load shedding#54
Conversation
Benchmark Results (Julia v1)Time benchmarks
Memory benchmarks
|
|
Follow-up DC island support is pushed in Multi-island root causeThe native DC stack assumed a single Fix
AC OPF is intentionally unchanged. Committed regressions
Validation
Disposable PGLib-OPF
|
|
Pushed follow-up commit Root causeThe multi-island change made each lightweight DC KKT parameter-Jacobian builder call Fix
Local verification
|
|
CI follow-up for
The GitHub benchmark confirms the accidental topology-scan regression is removed. Representative
The small residual sub-microsecond overhead is the energized-mask validation that preserves correctness for direct in-place topology mutations. |
|
Pushed The remaining table mixed two cases:
The follow-up stores the built model's effective reference count in a concrete internal Local verification:
|
|
CI benchmark follow-up for
All lightweight builders now match Removing that last delta would require a broader API decision: tracked mutation-aware vectors, or requiring all topology changes to go through an invalidating update function. I do not think that complexity or behavioral restriction is justified for a sub-microsecond bare-network helper cost. |
samtalki
left a comment
There was a problem hiding this comment.
Solid fix, and the test coverage here is genuinely good. A few things before it lands.
Scope: I'd split this. The actual #53 fix (psh ≤ max(d,0), keep signed d in power balance, fix psh=0 where d₊=0) is small, self-contained, and exactly what the issue asked for. The multi-island reference_buses work plus _DCTopologyCache and the _n_ref caching are a separate feature. They came out of the sweep that "surfaced separate edge cases," and they carry their own design decisions and two rounds of perf fixes for regressions the feature introduced. Bundling them means a one-line bug fix is now gated on reviewing a much larger, riskier change. I'd land the negative net demand fix on its own and put multi-island in a separate PR. If you'd rather keep them together that's fine, but say so in the title, since "Handle negative net demand" undersells what's actually in here.
Topology cache and threads. DCNetwork is an immutable struct, but topology_cache is a mutable struct that now gets written lazily on read. reference_buses, kkt_dims(net), kkt_indices(net), kkt(z, net, d), _factorize_B_r, all of them trigger _refresh_topology_cache! on first touch (and after any energized-edge change). No lock. So a DCNetwork that used to be safe to share across threads (truly immutable, read only) is now a data race if two threads first-touch it at the same time, or one reads while another flips sw/b. Either guard the refresh or document that a DCNetwork can no longer be shared across threads.
_n_ref is a second source of truth. kkt_dims(prob)/kkt_indices(prob) read the cached prob._n_ref, while kkt(z, net, d) and flatten_variables recompute _reference_buses(net) live. They agree as long as topology only changes through update_switching! (which rebuilds and refreshes _n_ref). Mutate prob.network.sw/b directly and they diverge: kkt_dims(prob) goes stale while the residual uses the fresh ref set, which is a confusing path to a dimension mismatch. It matches the existing "go through update_switching!" contract so it isn't wrong, but worth a comment pinning the invariant _n_ref == length(prob.cons.ref).
Minor:
_warn_negative_demandstill@warns now that negative net demand is a supported case rather than a failure. Anyone with legit negative net injection (embedded gen) gets warned on every build. Consider dropping it to@debug.kkt_indices(...).ηchanged from anIntto aUnitRange(length 1 in the single reference case). Fine inside the repo, just noting it for any external callers.
The negative demand path and the island math both have real KKT residual plus finite difference plus JVP/VJP coverage, which is the right way to do it. Nice work there.
60a7400 to
61bd883
Compare
|
Implemented the requested scope split.
Validation after the rewrite:
|
Corrected wording for load shedding cost vector and clarified demand constraints.
Summary
Addresses #53.
This PR fixes the IEEE300 DC OPF failure caused by negative net demand interacting with load-shedding bounds.
Root Cause
DC OPF previously imposed
0 <= psh <= d. IEEE300 contains buses with negative net demand, so those buses received contradictory shedding bounds:psh >= 0andpsh <= d < 0.Fix
d_plus = max(d, 0).din nodal power balance, so negative net demand remains an injection.psh = 0whered_plus = 0, because injections cannot be shed.update_demand!, solve post-processing, KKT residuals, analytical Jacobians, and allocation-light JVP/VJP paths.@warnto@debugnow that embedded generation / negative net demand is supported input.d = 0.update_demand!.AC OPF does not need the same code change: it has no load-shedding variable or shedding upper bound and already keeps signed load directly in nodal balance.
Follow-Up
The broader disconnected DC topology / multi-island work that was previously bundled here has been moved to draft follow-up #55. That PR is stacked on this one so the issue #53 fix can be reviewed and merged independently.
Validation
Pkg.test("PowerDiff")SITE_BUILD=true include("docs/make.jl")test, Documentationbuild, and Benchmarkbenchmark