From 25e94c69132fa56e976839613f5d01bd97ca0f71 Mon Sep 17 00:00:00 2001 From: Luke Kiernan Date: Mon, 24 Aug 2026 15:47:17 -0600 Subject: [PATCH 1/3] Fix transformer CM=2 magnetizing susceptance sign Under CM=2 a transformer record gives MAG1 as no-load loss in watts and MAG2 as the exciting current magnitude in device per-unit, positive by convention. The magnetizing branch is inductive, so the susceptance reconstructed from those must be the negative root, matching the already-negative susceptance a CM=1 record states directly (the convention the parser documents at the three-winding call site). Closes #36. Two adjacent fixes the sign bug sat on top of: - The zero-magnetizing guard compared MAG1/MAG2 against ZERO_IMPEDANCE_REACTANCE_THRESHOLD as a value rather than using it as a tolerance against zero, so `isapprox(0.0, 1e-4)` was false and a genuinely zero pair never took the branch. - That branch's warning reaches for "f_bus"/"t_bus", which only two-winding sub_data has; three-winding records name their buses "bus_primary"/"bus_secondary"/"bus_tertiary". With the guard fixed the three-winding warning lost its message to a swallowed KeyError, so the bus label is now built per winding count. The CM=2 case in test_openapi_transformer_discriminators.jl hand-derived its expectation from the implementation's own formula, sign included, so it encoded the bug; its expectation is corrected here. Co-Authored-By: Claude Opus 5 (1M context) --- src/pm_io/psse.jl | 23 ++++- ...test_openapi_transformer_discriminators.jl | 9 +- test/test_parse_psse.jl | 87 +++++++++++++++++++ 3 files changed, 112 insertions(+), 7 deletions(-) diff --git a/src/pm_io/psse.jl b/src/pm_io/psse.jl index 1d121b6..431aff0 100644 --- a/src/pm_io/psse.jl +++ b/src/pm_io/psse.jl @@ -1004,21 +1004,36 @@ function apply_tap_correction!( return windv_value end +# Two-winding records name their buses "f_bus"/"t_bus", three-winding ones +# "bus_primary"/"bus_secondary"/"bus_tertiary"; both reach the warning below, so reaching +# for the two-winding keys unconditionally costs the three-winding warning its message — +# the logging machinery swallows the KeyError and emits a stacktrace in its place. +function _transformer_bus_label(sub_data::Dict) + haskey(sub_data, "f_bus") && return "$(sub_data["f_bus"]) -> $(sub_data["t_bus"])" + return string( + sub_data["bus_primary"], " -> ", sub_data["bus_secondary"], + " -> ", sub_data["bus_tertiary"], + ) +end + # Base Power has a different key in sub_data depending on the number of windings function _transformer_mag_pu_conversion( transformer::Dict, sub_data::Dict, base_power::Float64, ) - if isapprox(transformer["MAG1"], ZERO_IMPEDANCE_REACTANCE_THRESHOLD) && - isapprox(transformer["MAG2"], ZERO_IMPEDANCE_REACTANCE_THRESHOLD) - @warn "Transformer $(sub_data["f_bus"]) -> $(sub_data["t_bus"]) has zero MAG1 and MAG2 values." + if isapprox(transformer["MAG1"], 0.0; atol = ZERO_IMPEDANCE_REACTANCE_THRESHOLD) && + isapprox(transformer["MAG2"], 0.0; atol = ZERO_IMPEDANCE_REACTANCE_THRESHOLD) + @warn "Transformer $(_transformer_bus_label(sub_data)) has zero MAG1 and MAG2 values." return 0.0, 0.0 else G_pu = PSSE_MICRO_UNIT_SCALE * transformer["MAG1"] / base_power mag_diff = transformer["MAG2"]^2 - G_pu^2 @assert mag_diff >= -ZERO_IMPEDANCE_REACTANCE_THRESHOLD - B_pu = sqrt(max(0.0, mag_diff)) + # The magnetizing branch is inductive: MAG2 under CM=2 is the exciting current + # magnitude (positive by convention), so the susceptance it implies is the + # negative root, matching the already-negative MAG2 a CM=1 record supplies. + B_pu = -sqrt(max(0.0, mag_diff)) return G_pu, B_pu end end diff --git a/test/test_openapi_transformer_discriminators.jl b/test/test_openapi_transformer_discriminators.jl index 7635ab4..5a54a12 100644 --- a/test/test_openapi_transformer_discriminators.jl +++ b/test/test_openapi_transformer_discriminators.jl @@ -111,13 +111,16 @@ end @test d["br_x"] ≈ 0.05626483087470335 # CM=2: MAG1 (80000.0 W) is no-load loss, MAG2 (0.005 pu) is exciting current; - # G_pu = 1e-6*MAG1/base_power, B_pu = sqrt(MAG2^2 - G_pu^2). + # G_pu = 1e-6*MAG1/base_power, B_pu = -sqrt(MAG2^2 - G_pu^2). The root is negative + # because the magnetizing branch is inductive: MAG2 is a current magnitude here, + # whereas a CM=1 record states the same susceptance directly and already negative. g_pu = 1e-6 * 80000.0 / d["base_power"] @test g_pu ≈ 0.001 - b_pu = sqrt(0.005^2 - g_pu^2) - @test b_pu ≈ 0.004898979485566357 + b_pu = -sqrt(0.005^2 - g_pu^2) + @test b_pu ≈ -0.004898979485566357 @test d["g_fr"] ≈ g_pu @test d["b_fr"] ≈ b_pu + @test d["b_fr"] < 0 # tap: WINDV1/WINDV2 * (bus_J_base_kv/bus_I_base_kv) * (winding1_nominal_voltage / # winding2_nominal_voltage); NOMV1=0 -> winding1_nominal_voltage = bus_I_base_kv diff --git a/test/test_parse_psse.jl b/test/test_parse_psse.jl index 9a127cd..9cef3a5 100644 --- a/test/test_parse_psse.jl +++ b/test/test_parse_psse.jl @@ -154,3 +154,90 @@ end @test length(shunt_101["y_increment"]) == 1 @test shunt_101["initial_status"] == [0] end + +@testset "PSSE transformer CM=2 magnetizing susceptance is inductive" begin + # Under CM=2 a transformer record gives MAG1 as no-load loss in watts and MAG2 as the + # exciting current magnitude in device per-unit — a positive number by convention. The + # magnetizing branch is inductive, so the susceptance derived from it must be the + # negative root, matching the already-negative susceptance a CM=1 record supplies + # directly. + raw = read_fixture(FOURTEEN_BUS_FIXTURE) + + # The fixture's transformers are all CM=1 with zero MAG1/MAG2. Flip one two-winding + # and one three-winding record to CM=2 and give them a loss/exciting-current pair. + # Record line 1 is `I, J, K, CKT, CW, CZ, CM, MAG1, MAG2, NMETR, NAME, ...`. + mag1_watts, mag2_pu = 3.0e4, 5.0e-3 + cm2 = replace( + raw, + " 109, 104, 0,'1 ',1,1,1, 0.00000E+0, 0.00000E+0,2,'TRAFO 2W 3 '" => + " 109, 104, 0,'1 ',1,1,2, 3.00000E+4, 5.00000E-3,2,'TRAFO 2W 3 '", + " 109, 104, 107,'1 ',1,1,1, 0.00000E+0, 0.00000E+0,2,'TRAFO 3W 2 '" => + " 109, 104, 107,'1 ',1,1,2, 3.00000E+4, 5.00000E-3,2,'TRAFO 3W 2 '", + ) + @test cm2 != raw + pm_data = parse_file(IOBuffer(cm2); filetype = "raw") + + # SBASE1-2 is 100.0 for both records, so G is watts scaled to that base and B closes + # the right triangle against the exciting current. + expected_g = 1e-6 * mag1_watts / 100.0 + expected_b = -sqrt(mag2_pu^2 - expected_g^2) + @test expected_b < 0 + + branch = only( + v for v in values(pm_data["branch"]) if + get(get(v, "ext", Dict()), "psse_name", "") == "TRAFO 2W 3 " + ) + @test branch["g_fr"] ≈ expected_g + @test branch["b_fr"] ≈ expected_b + @test branch["b_fr"] < 0 + + transformer_3w = only( + v for v in values(pm_data["3w_transformer"]) if + get(get(v, "ext", Dict()), "psse_name", "") == "TRAFO 3W 2 " + ) + @test transformer_3w["g"] ≈ expected_g + @test transformer_3w["b"] ≈ expected_b + @test transformer_3w["b"] < 0 +end + +@testset "PSSE transformer CM=2 zero MAG1/MAG2 warns on both winding counts" begin + # The zero check guards against a magnetizing branch with nothing to derive. It only + # runs under CM=2, so flip the same two records the sign test uses but leave their + # MAG1/MAG2 at the fixture's zeros. The three-winding record is the one that matters: + # its sub_data names buses "bus_primary"/"bus_secondary"/"bus_tertiary", so a warning + # reaching for "f_bus" would throw rather than warn. + raw = read_fixture(FOURTEEN_BUS_FIXTURE) + cm2 = replace( + raw, + " 109, 104, 0,'1 ',1,1,1, 0.00000E+0, 0.00000E+0,2,'TRAFO 2W 3 '" => + " 109, 104, 0,'1 ',1,1,2, 0.00000E+0, 0.00000E+0,2,'TRAFO 2W 3 '", + " 109, 104, 107,'1 ',1,1,1, 0.00000E+0, 0.00000E+0,2,'TRAFO 3W 2 '" => + " 109, 104, 107,'1 ',1,1,2, 0.00000E+0, 0.00000E+0,2,'TRAFO 3W 2 '", + ) + @test cm2 != raw + + # Collect the records rather than using @test_logs: a message that fails to build is + # still reported as a warning, and match_mode = :any accepts that stand-in, so assert + # on the rendered message text directly. + logs, pm_data = Test.collect_test_logs() do + parse_file(IOBuffer(cm2); filetype = "raw") + end + messages = [string(r.message) for r in logs if r.level == Logging.Warn] + @test count(m -> occursin("has zero MAG1 and MAG2 values", m), messages) == 2 + @test any(m -> occursin("Transformer 109 -> 104 has zero", m), messages) + @test any(m -> occursin("Transformer 109 -> 104 -> 107 has zero", m), messages) + + branch = only( + v for v in values(pm_data["branch"]) if + get(get(v, "ext", Dict()), "psse_name", "") == "TRAFO 2W 3 " + ) + @test branch["g_fr"] == 0.0 + @test branch["b_fr"] == 0.0 + + transformer_3w = only( + v for v in values(pm_data["3w_transformer"]) if + get(get(v, "ext", Dict()), "psse_name", "") == "TRAFO 3W 2 " + ) + @test transformer_3w["g"] == 0.0 + @test transformer_3w["b"] == 0.0 +end From 8680f70a42b4e711f6a6430f9fbcecd932cadece Mon Sep 17 00:00:00 2001 From: Luke Kiernan Date: Mon, 24 Aug 2026 16:05:27 -0600 Subject: [PATCH 2/3] formatting --- test/test_parse_psse.jl | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/test/test_parse_psse.jl b/test/test_parse_psse.jl index 9cef3a5..0063570 100644 --- a/test/test_parse_psse.jl +++ b/test/test_parse_psse.jl @@ -169,10 +169,8 @@ end mag1_watts, mag2_pu = 3.0e4, 5.0e-3 cm2 = replace( raw, - " 109, 104, 0,'1 ',1,1,1, 0.00000E+0, 0.00000E+0,2,'TRAFO 2W 3 '" => - " 109, 104, 0,'1 ',1,1,2, 3.00000E+4, 5.00000E-3,2,'TRAFO 2W 3 '", - " 109, 104, 107,'1 ',1,1,1, 0.00000E+0, 0.00000E+0,2,'TRAFO 3W 2 '" => - " 109, 104, 107,'1 ',1,1,2, 3.00000E+4, 5.00000E-3,2,'TRAFO 3W 2 '", + " 109, 104, 0,'1 ',1,1,1, 0.00000E+0, 0.00000E+0,2,'TRAFO 2W 3 '" => " 109, 104, 0,'1 ',1,1,2, 3.00000E+4, 5.00000E-3,2,'TRAFO 2W 3 '", + " 109, 104, 107,'1 ',1,1,1, 0.00000E+0, 0.00000E+0,2,'TRAFO 3W 2 '" => " 109, 104, 107,'1 ',1,1,2, 3.00000E+4, 5.00000E-3,2,'TRAFO 3W 2 '", ) @test cm2 != raw pm_data = parse_file(IOBuffer(cm2); filetype = "raw") @@ -209,10 +207,8 @@ end raw = read_fixture(FOURTEEN_BUS_FIXTURE) cm2 = replace( raw, - " 109, 104, 0,'1 ',1,1,1, 0.00000E+0, 0.00000E+0,2,'TRAFO 2W 3 '" => - " 109, 104, 0,'1 ',1,1,2, 0.00000E+0, 0.00000E+0,2,'TRAFO 2W 3 '", - " 109, 104, 107,'1 ',1,1,1, 0.00000E+0, 0.00000E+0,2,'TRAFO 3W 2 '" => - " 109, 104, 107,'1 ',1,1,2, 0.00000E+0, 0.00000E+0,2,'TRAFO 3W 2 '", + " 109, 104, 0,'1 ',1,1,1, 0.00000E+0, 0.00000E+0,2,'TRAFO 2W 3 '" => " 109, 104, 0,'1 ',1,1,2, 0.00000E+0, 0.00000E+0,2,'TRAFO 2W 3 '", + " 109, 104, 107,'1 ',1,1,1, 0.00000E+0, 0.00000E+0,2,'TRAFO 3W 2 '" => " 109, 104, 107,'1 ',1,1,2, 0.00000E+0, 0.00000E+0,2,'TRAFO 3W 2 '", ) @test cm2 != raw From adcb77a20430609f09626a7cc273398380967d29 Mon Sep 17 00:00:00 2001 From: Luke Kiernan Date: Mon, 24 Aug 2026 16:09:42 -0600 Subject: [PATCH 3/3] trim some comments --- src/pm_io/psse.jl | 4 +--- test/test_openapi_transformer_discriminators.jl | 4 +--- test/test_parse_psse.jl | 7 ++----- 3 files changed, 4 insertions(+), 11 deletions(-) diff --git a/src/pm_io/psse.jl b/src/pm_io/psse.jl index 431aff0..d0e5c15 100644 --- a/src/pm_io/psse.jl +++ b/src/pm_io/psse.jl @@ -1005,9 +1005,7 @@ function apply_tap_correction!( end # Two-winding records name their buses "f_bus"/"t_bus", three-winding ones -# "bus_primary"/"bus_secondary"/"bus_tertiary"; both reach the warning below, so reaching -# for the two-winding keys unconditionally costs the three-winding warning its message — -# the logging machinery swallows the KeyError and emits a stacktrace in its place. +# "bus_primary"/"bus_secondary"/"bus_tertiary"; both reach the warning below. function _transformer_bus_label(sub_data::Dict) haskey(sub_data, "f_bus") && return "$(sub_data["f_bus"]) -> $(sub_data["t_bus"])" return string( diff --git a/test/test_openapi_transformer_discriminators.jl b/test/test_openapi_transformer_discriminators.jl index 5a54a12..c119840 100644 --- a/test/test_openapi_transformer_discriminators.jl +++ b/test/test_openapi_transformer_discriminators.jl @@ -111,9 +111,7 @@ end @test d["br_x"] ≈ 0.05626483087470335 # CM=2: MAG1 (80000.0 W) is no-load loss, MAG2 (0.005 pu) is exciting current; - # G_pu = 1e-6*MAG1/base_power, B_pu = -sqrt(MAG2^2 - G_pu^2). The root is negative - # because the magnetizing branch is inductive: MAG2 is a current magnitude here, - # whereas a CM=1 record states the same susceptance directly and already negative. + # G_pu = 1e-6*MAG1/base_power, B_pu = -sqrt(MAG2^2 - G_pu^2). g_pu = 1e-6 * 80000.0 / d["base_power"] @test g_pu ≈ 0.001 b_pu = -sqrt(0.005^2 - g_pu^2) diff --git a/test/test_parse_psse.jl b/test/test_parse_psse.jl index 0063570..34446cb 100644 --- a/test/test_parse_psse.jl +++ b/test/test_parse_psse.jl @@ -156,11 +156,8 @@ end end @testset "PSSE transformer CM=2 magnetizing susceptance is inductive" begin - # Under CM=2 a transformer record gives MAG1 as no-load loss in watts and MAG2 as the - # exciting current magnitude in device per-unit — a positive number by convention. The - # magnetizing branch is inductive, so the susceptance derived from it must be the - # negative root, matching the already-negative susceptance a CM=1 record supplies - # directly. + # Under CM=2 a transformer record gives MAG1 as a positive number by convention, when + # the magnetizing branch is inductive (negative susceptance). raw = read_fixture(FOURTEEN_BUS_FIXTURE) # The fixture's transformers are all CM=1 with zero MAG1/MAG2. Flip one two-winding