Match PSS(e) zero-impedance branch handling (per-branch L2 norm)#329
Open
luke-kiernan wants to merge 4 commits into
Open
Match PSS(e) zero-impedance branch handling (per-branch L2 norm)#329luke-kiernan wants to merge 4 commits into
luke-kiernan wants to merge 4 commits into
Conversation
Fixes items 1 and 2 of #322. ZIBR `get_reduction` previously scanned the assembled Ybus off-diagonals and merged a bus pair only when `real(Y[i,j]) == 0 && abs(imag(Y[i,j])) >= threshold`. This diverged from PSS(e): 1. It gated on `iszero(real(Y))` instead of the L2 norm `abs(Y)`, so a near-short with a small resistive part was skipped. 2. It tested the *summed* parallel-branch entry, so a single near-zero- impedance branch (e.g. 1e4im) hidden by an anti-parallel member that cancels it in the sum (e.g. -10im) was not merged. Rework `get_reduction` to iterate the direct and parallel branch maps (the only arc sources at ZIBR time, the first reduction) and test each branch's series admittance. An arc merges when either any individual non-transformer branch clears the threshold (PSS(e)'s per-branch rule) or the combined entry does (item 3 — kept as the numerically robust coupling measure, since whether PSS(e)/correctness wants a merge there is unresolved). The union is a superset of the old combined-only test, so item 3 behavior is unchanged. The merge `arc_key` is now obtained directly as the branch-map key rather than reverse-looked-up from Ybus indices via `_find_zero_impedance_arc_key`; the tuple is identical, so `removed_arcs` and the merge maps are unchanged. This also drops the `_build_transformer_arc_set` scan, with transformer exclusion kept via `_is_transformer`/`_any_transformer`. Adds regression tests for items 1, 2, 3, and the all-below-threshold case. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Updates ZeroImpedanceBranchReduction (ZIBR) to more closely match PSS(e) by detecting zero-impedance merges using per-branch admittance magnitude (L2 norm), while still optionally merging based on the combined off-diagonal coupling for parallel groups.
Changes:
- Reworks ZIBR detection to iterate direct/parallel branch maps and apply per-branch
abs(y) >= thresholdlogic (plus a combined-entry fallback for parallel groups). - Updates ZIBR documentation to reflect the new per-branch + combined-entry criteria.
- Adds regression tests covering near-short cancellation in parallel groups, combined-entry-only merges, and L2 detection with nonzero resistance.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/apply_zero_impedance_reduction.jl |
Implements per-arc/per-branch ZIBR detection and merge logic. |
src/zero_impedance_branch_reduction.jl |
Updates docstring/spec to describe the revised ZIBR rule. |
test/test_ybus_reductions.jl |
Adds targeted regression tests for the updated ZIBR behavior. |
💡 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.
|
Per CoPilot review on #329: the merge predicate called `ybus_branch_entries`, re-emitting its `r == x == 0` warning (assembly already emitted it) and rebuilding the 2x2; and the parallel combined term used `ybus_branch_entries(group, nr)`, which ignores `min_x_eps`. Compute `Y_l = 1/(r + x im)` directly from `(r, x)` with the same `min_x_eps` substitution, and form the combined entry as `sum(Y_l)` (members are symmetric). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
2 comments:
edit: first round of behavioral changes fixed |
The per-branch L2 criterion now merges zero-impedance branches that the old `iszero(real(Y))` gate skipped (e.g. 228 buses on CATS), so the default Ybus no longer matches MATPOWER's `makeYbus`, which does no bus merging. Disable the auto-applied ZeroImpedanceBranchReduction (susceptance_threshold = Inf) in the MATPOWER comparison tests so they validate raw assembly against an unreduced baseline; bus-merging behavior is covered in test_ybus_reductions.jl. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The Base_Eastern_Interconnect_515GW PSS(e) export has no "ZERO IMPEDANCE LINE CONNECTED BUSES" section, so its baseline is unreduced. The per-branch L2 criterion now merges 28 low-impedance branches PSS(e) left in place here, so build with susceptance_threshold = Inf to compare unreduced matrices. The other PSS(e) cases do list zero-impedance groups and still validate that PNM matches PSS(e)'s merges, so they are left unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
luke-kiernan
force-pushed
the
fix/zibr-per-branch-l2
branch
from
4b86712 to
c919d37
Compare
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.
Fixes items 1 and 2 of #322.
Problem
ZeroImpedanceBranchReduction'sget_reductionscanned the assembledYbusoff-diagonals and merged a bus pair only when
real(Y[i,j]) == 0 && abs(imag(Y[i,j])) >= susceptance_threshold, diverging fromPSS(e):
iszero(real(Y))gate skipped near-shorts with a smallresistive part. PSS(e) uses
abs(y) >= threshold.combined parallel entry, so a single near-zero-impedance branch hidden by an
anti-parallel member that cancels it in the sum was not merged.
Change
get_reductionnow iterates the direct and parallel branch maps and tests eachbranch's series admittance. An arc is treated as zero-impedance when either
some individual non-transformer branch clears the threshold (PSS(e)'s per-branch
L2 rule — fixes items 1 & 2) or the combined off-diagonal entry clears it.
Item 3 (two sub-threshold branches whose sum clears the threshold) is left
merging via the combined term, pending confirmation of PSS(e)'s behavior
there.
The merge
arc_keyis read directly as the branch-map key (identical to the oldreverse-lookup), so
removed_arcsand the merge maps are unchanged. The loopiterates only the direct and parallel maps; it no longer consults
series_branch_map, which is empty when ZIBR runs (it is always the firstreduction).
Tests
makeYbus, unreduced) and the Eastern PSS(e) export (nozero-impedance section) are compared against the unreduced PNM matrix
(
susceptance_threshold = Inf); the other PSS(e) cases keep theirzero-impedance groups and still validate that PNM matches PSS(e)'s merges.
🤖 Generated with Claude Code