fix: PSSE v35 export of systems with no non-transformer branches (#361)#394
Merged
Conversation
When a system has no Lines/MonitoredLines/DiscreteControlledACBranches,
`get_branches_with_numbers` returned an untyped comprehension, so inference
widened the bus-number container type to `Tuple{Int, Int, Vararg{Int}}`. The
resulting empty `branch_name_mapping` matched no `serialize_component_ids`
method and the v35 export threw a `MethodError`.
Pin the comprehension element type to `Tuple{PSY.ACBranch, Tuple{Int, Int}}`
so the empty case keeps concrete `Tuple{Int, Int}` container ids, which the
existing 2-tuple `serialize_component_ids` dispatch handles.
Add two regression tests mirroring the issue's repros:
- v35 export of a programmatic Line missing RATE4..RATE12 ext keys (the
numeric-default path fixed in #360), asserting extra ratings export as 0.0.
- v35 export of a system with no non-transformer branches.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Performance ResultsPrecompile Time
Solve TimePolar AC
Rectangular CI
Mixed CPB
DC
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a PSS/E v35 export failure when exporting systems that have zero non-transformer branches, caused by type-widening in get_branches_with_numbers leading to a serialize_component_ids dispatch miss. It also adds regression tests covering both the empty-branches case and the previously-reported v35 RATE4–RATE12 missing-ext-keys case.
Changes:
- Make
get_branches_with_numbersreturn a concretely-typedVector{Tuple{PSY.ACBranch, Tuple{Int, Int}}}even when empty to avoid type-widening. - Add regression tests ensuring v35 export succeeds for (1) a programmatically-built
Linemissing RATE4–RATE12 ext keys and (2) a system with no non-transformer branches.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/psse_export.jl |
Pins the branch/bus-number tuple vector element type to prevent empty-collection type widening and downstream dispatch failures. |
test/test_psse_export.jl |
Adds two regression testsets reproducing issue #361 scenarios and asserting successful export + expected metadata/RAW output. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
mcllerena
approved these changes
Jun 16, 2026
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.
Summary
Fixes the second (still-open) half of #361: exporting a v35 PSS/e file from a system with no non-transformer branches threw
Root cause
get_branches_with_numbersreturned an untyped comprehension. With zero branches, inference widened the bus-number container type toTuple{Int, Int, Vararg{Int}}, so the emptybranch_name_mappingdict matched none of the concreteserialize_component_idsdispatches.Fix
Pin the comprehension element type to
Tuple{PSY.ACBranch, Tuple{Int, Int}}so the empty case keeps concreteTuple{Int, Int}container ids, which the existing 2-tupleserialize_component_idsmethod handles. (Matches the issue's suggested "concrete element type" fix and Sienna's concrete-container convention.)Tests
Two regression testsets in
test_psse_export.jl, mirroring the issue's minimal repros:0.0.branch_name_mappingis empty.Both fail on the prior code (they threw before producing files) and pass with this change. Full local
test_psse_export.jlsuite passes.Closes #361.
🤖 Generated with Claude Code