Repository navigation
Keep each 3W winding's ratings instead of their minimum - #91
luke-kiernan wants to merge 3 commits into
Conversation
A winding's rating was min(RATA, RATB, RATC), but zero means no limit, so one unset rating turned the winding unbounded and lost its real limits. Map each winding's ratings to rating, rating_b and rating_c, as a 2W transformer does. Drop the 3W "rating" key, which nothing read. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
🟡 Changes recommended
The output regression checks only :rating, leaving B/C output omissions undetected; it needs assertions for each winding’s B/C values and for zero source ratings remaining unset.
1 open issue
What changed in this PR
This PR preserves each three-winding transformer's A/B/C ratings instead of taking their minimum, preventing unset B or C ratings from erasing valid A limits.
Changes:
- Maps legacy and v35 winding ratings individually and removes the unused aggregate rating.
- Exports per-winding B/C ratings to OpenAPI circuits.
- Adds parsing regressions and an output check for the previously unbounded winding limits.
| File | Description |
|---|---|
test/test_parse_transformers.jl |
Tests legacy/v35 rating mappings and finite output limits. |
src/pm_io/psse.jl |
Preserves individual winding ratings, omitting zero B/C values. |
src/openapi/branch.jl |
Exports winding B/C ratings and updates the units comment. |
Review effort: Balanced
| rating_b = _get_rating(name, d, "rating_b_primary"), | ||
| rating_c = _get_rating(name, d, "rating_c_primary"), |
The output test only checked rating, so dropping rating_b or rating_c would have passed. Compare all three ratings on each winding's own circuit, including a case where every rating is distinct. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
| sub_data["rating_secondary"], | ||
| sub_data["rating_tertiary"], | ||
| ) | ||
| rating_keys = i -> ("RATE$(i)1", "RATE$(i)2", "RATE$(i)3") |
There was a problem hiding this comment.
For Lines and TwoWinding Transformers we add ratings 4-12 into the ext if the source version is PSSE_TWELVE_RATING_VERSIONS. We should probably match that here for 3wT
| "Unsupported PSS(R)E source version: $(pm_data["source_version"])", | ||
| ) | ||
| end | ||
| # A zero rating means no limit, so it must not win over the ones that are |
There was a problem hiding this comment.
This comment references the old code bug. It should describe the corrected code only
|
Suggestion: Write unset (zero) ratings the same way everywhere the dict is built, by never writing the key, through one helper used by the line, 2W, 3W and MATPOWER parsers. |
Lines and 2W transformers keep these in twelve-rating versions, but 3W transformers dropped them. Also rewrite the rating comment to describe the code as it is now. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@m-bossart thanks for the review. Addressed in 9db60ea:
I left the suggestion about writing unset ratings the same way everywhere for #94, since it reaches beyond this PR. |
Summary
A three-winding transformer's winding rating was
min(RATA, RATB, RATC). Zero means "no limit" in PSS/E, so one unset rating made the minimum 0, which the output then read as unbounded. Infrankenstein_70.raw, the file gives 336, 268 and 84 MVA withRATC = 0, and all three windings came out as 1e6 MVA.Each winding now keeps its own ratings, as a two-winding transformer does: rating A is
rating, and nonzero B and C arerating_bandrating_c. v35'sRATEn1-RATEn3follow the same mapping.Output changes
rating_bandrating_c, which were always unset."rating"key is gone from the PowerModels dict; nothing read it.ext, as they are for lines and 2W transformers.Writing unset ratings the same way across the parsers is left to #94.
Testing
frankenstein_70.raw. It fails without the fix.🤖 Generated with Claude Code