Skip to content

handle reverse parallel arc#317

Open
m-bossart wants to merge 1 commit into
mainfrom
mb/reverse-parallel-arc
Open

handle reverse parallel arc#317
m-bossart wants to merge 1 commit into
mainfrom
mb/reverse-parallel-arc

Conversation

@m-bossart
Copy link
Copy Markdown
Contributor

No description provided.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 2, 2026

Performance Results

Precompile Time

Main This Branch Delta
2.243 s 2.249 s +0.3%

Execution Time

Test Main This Branch Delta
matpower_ACTIVSg2000_sys-Build PTDF First 2.249 s 2.366 s +5.2%
matpower_ACTIVSg2000_sys-Build PTDF Second 93.7 ms 77.5 ms -17.3%
matpower_ACTIVSg2000_sys-Build Ybus First 11.7 ms 11.7 ms +0.1%
matpower_ACTIVSg2000_sys-Build Ybus Second 10.0 ms 11.2 ms +12.2%
matpower_ACTIVSg2000_sys-Build LODF First 141.3 ms 154.5 ms +9.3%
matpower_ACTIVSg2000_sys-Build LODF Second 620.7 ms 212.8 ms -65.7%
matpower_ACTIVSg2000_sys-Build VirtualMODF First 2.162 s 4.089 s +89.1%
matpower_ACTIVSg2000_sys-Build VirtualMODF Second 101.0 ms 1.071 s +959.7%
matpower_ACTIVSg2000_sys-VirtualMODF Query 10 rows 512.1 ms 516.6 ms +0.9%
matpower_ACTIVSg2000_sys-Radial network reduction First 546.9 ms 543.9 ms -0.6%
matpower_ACTIVSg2000_sys-Radial network reduction Second 0.8 ms 0.7 ms -17.4%
matpower_ACTIVSg2000_sys-Degree two network reduction First 1.871 s 1.936 s +3.4%
matpower_ACTIVSg2000_sys-Degree two network reduction Second 1.3 ms 1.1 ms -12.7%
Base_Eastern_Interconnect_515GW-Build Ybus First 834.5 ms 817.6 ms -2.0%
Base_Eastern_Interconnect_515GW-Build Ybus Second 630.1 ms 933.4 ms +48.1%
Base_Eastern_Interconnect_515GW-Radial network reduction First 39.2 ms 44.8 ms +14.3%
Base_Eastern_Interconnect_515GW-Radial network reduction Second 45.9 ms 45.5 ms -0.8%
Base_Eastern_Interconnect_515GW-Degree two network reduction First 385.1 ms 401.1 ms +4.2%
Base_Eastern_Interconnect_515GW-Degree two network reduction Second 48.3 ms 41.4 ms -14.2%

@jd-lara
Copy link
Copy Markdown
Member

jd-lara commented Jun 2, 2026

Code review

Found 1 issue:

  1. Anti-parallel branches are merged into one BranchesParallel group keyed by reverse_new_arc, but the group's orientation invariant is not preserved. The incoming branch is stored under the reverse direction without flipping its admittance block. This is correct for the DC path (BA/ABA/PTDF/LODF use the orientation-independent scalar get_series_susceptance, which is why the fix works there), but the AC equivalent path sums each member's 2x2 Y-block positionally and explicitly assumes all members share orientation:

function ybus_branch_entries(
parallel_br::AbstractBranchesParallel;
min_x_eps::Float64 = ZERO_IMPEDANCE_X_EPSILON,
)
arc = get_arc_tuple(first(parallel_br))
Y11 = Y12 = Y21 = Y22 = zero(YBUS_ELTYPE)
for br in parallel_br
# All branches in BranchesParallel are have the same orientation when constructed in add_to_branch_maps!
(y11, y12, y21, y22) = ybus_branch_entries(br)
Y11 += y11
Y12 += y12
Y21 += y21
Y22 += y22
end
return (Y11, Y12, Y21, Y22)
end

The new collision cases violate that invariant by placing a branch oriented new_arc = (A,B) into a group keyed reverse_new_arc = (B,A):

existing = pop!(nr.direct_branch_map, reverse_new_arc)
@debug "Bus merge created anti-parallel collision: remapped arc $new_arc conflicts with existing $reverse_new_arc; promoting $(get_name(existing)) and $(get_name(val)) to a parallel group under $reverse_new_arc."
if haskey(nr.parallel_branch_map, reverse_new_arc)
_push_parallel_branch!(nr.parallel_branch_map, reverse_new_arc, existing)
_push_parallel_branch!(nr.parallel_branch_map, reverse_new_arc, val)
else
nr.parallel_branch_map[reverse_new_arc] =
_make_parallel_branch_pair(existing, val, reverse_new_arc)
end
elseif haskey(nr.parallel_branch_map, reverse_new_arc)
@debug "Bus merge created anti-parallel collision: remapped arc $new_arc conflicts with existing parallel group at $reverse_new_arc; adding $(get_name(val)) to that group."
_push_parallel_branch!(nr.parallel_branch_map, reverse_new_arc, val)
else
nr.direct_branch_map[new_arc] = val
end

(same pattern in the parallel→reverse cases at L1676-L1685). For symmetric lines Y11=Y22 and Y12=Y21, so the positional sum is unaffected. For asymmetric branches (transformers, phase shifters), Y11≠Y22 and Y12≠Y21, so the reverse-oriented member's tap/shunt admittance lands on the wrong port and get_equivalent_physical_branch_parameters returns an incorrect equivalent Ybus. Consider swapping the reverse member's (Y11,Y12,Y21,Y22) to (Y22,Y21,Y12,Y11) before grouping, or normalizing orientation, so the L478 invariant continues to hold.

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants