Fix transformer CM=2 magnetizing susceptance sign

[luke-kiernan]

Closes #36.

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 reconstructed from those must be the negative root. The parser took the positive one, flipping the sign relative to a CM=1 record, which states the same susceptance directly and already negative — the convention the parser itself documents at the three-winding call site (psse.jl:1703-1704).

The fix is one line in _transformer_mag_pu_conversion, which is reached only from the two CM=2 branches (two-winding and three-winding), so CM=1 is untouched.

Two adjacent fixes the sign bug sat on top of

• Zero-magnetizing guard. It compared MAG1/MAG2 against ZERO_IMPEDANCE_REACTANCE_THRESHOLD as a value rather than using it as a tolerance against zero. isapprox(0.0, 1e-4) is false with default kwargs, so a genuinely zero pair never took the branch — it fell through to the arithmetic, which happened to yield the same numbers without the intended warning.
• Three-winding warning message. That branch's warning reaches for sub_data["f_bus"]/["t_bus"], which only two-winding records have; three-winding ones name their buses bus_primary/bus_secondary/bus_tertiary. Fixing the guard exposed this: the three-winding warning lost its message to a KeyError that the logging machinery swallows into an error report. The parse does not abort, but the warning is gone. The bus label is now built per winding count.

Tests

• test_parse_psse.jl — the fixture's transformers are all CM=1 with zero MAG1/MAG2, so two new testsets flip one two-winding and one three-winding record to CM=2 via the existing read_fixture/replace pattern. The first pins g/b against hand-computed values and asserts the susceptance is negative; the second leaves MAG1/MAG2 at zero to exercise the guard and asserts both warning messages render with the right bus numbers. Together these cover both CM=2 call sites — the pre-existing discriminator test only reaches the two-winding one.
• test_openapi_transformer_discriminators.jl — the existing CM=2 case hand-derived its expected b_fr from the implementation's own formula, sign included, so it encoded the bug rather than independently confirming it. Expectation corrected, with an explicit b_fr < 0.

Each of the three fixes was verified to fail the suite when reverted individually. Full suite: 2419 pass, 0 fail.

🤖 Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[Copilot]

Pull request overview

Fixes the PSS/E RAW transformer parser’s CM=2 magnetizing branch reconstruction so the derived susceptance is inductive (negative), consistent with CM=1 convention, and hardens the CM=2 “zero MAG1/MAG2” warning path so it reliably emits a meaningful message for both two- and three-winding transformers.

Changes:

• Corrected CM=2 magnetizing susceptance sign by taking the negative square root in _transformer_mag_pu_conversion.
• Fixed the “zero MAG1/MAG2” guard to use ZERO_IMPEDANCE_REACTANCE_THRESHOLD as a tolerance around 0.0, and ensured warning messages render for both 2W and 3W transformers via _transformer_bus_label.
• Added/updated tests to independently assert the negative susceptance expectation and to cover the zero-magnetizing warning behavior for both winding counts.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
src/pm_io/psse.jl	Fixes CM=2 susceptance sign, corrects zero guard tolerance usage, and makes warning bus labeling work for both 2W/3W transformer shapes.
test/test_parse_psse.jl	Adds fixture-based tests covering CM=2 sign behavior (2W + 3W) and the zero MAG1/MAG2 warning message rendering for both winding counts.
test/test_openapi_transformer_discriminators.jl	Updates the CM=2 expected b_fr sign and adds an explicit assertion that the susceptance is negative.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[luke-kiernan]

One detail from Claude that might be worth checking:

The CM=2 paths assign G_pu/B_pu straight through while CM=1 divides by mva_ratio_12. I believe that's correct since the helper already divides by the device base internally, but I only checked the sign here.

[mcllerena]

LGTM, this matches the patch done on the PSY side for a related problem.
Fixing CI passing is missing?

[luke-kiernan]

Fixing CI passing is missing?

Not sure what you mean. CI tests are passing, just documentation is failing (with an error that I don't recognize).