Units migration: adopt unitful PowerSystems getter API#308
Open
luke-kiernan wants to merge 6 commits into
Open
Units migration: adopt unitful PowerSystems getter API#308luke-kiernan wants to merge 6 commits into
luke-kiernan wants to merge 6 commits into
Conversation
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace the placeholder `Float64` getter target with explicit unit systems: `SU` (system base) for impedances, admittances, and shunts used in matrix construction, and `DU` (device base) for equivalent ratings, which must work on detached branches. Thread `units::IS.AbstractUnitSystem` through the `get_series_susceptance` accessors (PSY now requires it). Pin InfrastructureSystems/PowerSystems (and test deps PowerSystemCaseBuilder/ PowerFlowFileParser) to their units-aware branches via `[sources]` so CI can resolve the as-yet-unreleased upstream changes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Member
|
Don't we need PSY6 for this work to make sense? |
Collaborator
Author
Yes. I didn't see a |
Member
|
@copilot merge main onto this branch and correct the conflicts |
Contributor
Merged |
Re-point [sources] from the units feature branches to the next-release
branches now that they are merged upstream:
- InfrastructureSystems: lk/units-domain-agnostic-is4 -> IS4
- PowerSystems: lk/units-fold-psu -> psy6
- PowerSystemCaseBuilder lk/units-table-driven -> psy6
- PowerFlowFileParser: fork lk/units-fixes -> NREL-Sienna main
(the fix is merged to main but predates the only release, v0.1.0)
Fix two breakages surfaced by the bump to psy6:
- virtual_lodf_calculations.jl: pass PSY.SU to get_series_susceptance
in _extract_branch_susceptances_by_arc (missed call sites; the
accessor now requires an explicit unit system).
- test_arc_types_and_reductions.jl: attach the MixedBranchesParallel
test's Line/TapTransformer to a System so the system-base getters in
ybus_branch_entries resolve (psy6 now throws on detached components).
- test_connectivity.jl: relax the VirtualPTDF/VirtualLODF cross-method
equality checks from == to isapprox; the on-demand paths now differ
by ULPs on the IS4/psy6 stack.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Performance ResultsPrecompile Time
Execution Time
|
Finish threading the unit-aware PSY getter API through the subsystems the original migration missed, surfaced by the bump to the psy6/IS4 release branches: - connectivity_checks.jl: reimplement find_connected_components with PNM's own union-find instead of PowerModels.calc_connected_components, which PowerSystems no longer re-exports (PowerModels dependency dropped). - common.jl, network_modification.jl: pass PSY.SU to get_series_susceptance and get_impedance_active/reactive_power in the contingency/MODF delta computations (all operate on system-base, system-attached components). - Ybus.jl, BranchesSeries.jl: the synthetic ward-equivalent GenericArcImpedance is detached by design and stores system-base impedance; read it back with device base (DU, identity) since system base would need a system base power a detached component cannot resolve. - BranchesParallel.jl: document why get_impedance_averaged_rating uses system base (consistent weighting across the group; requires attached branches). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- test_connectivity.jl: compare the dense PTDF/LODF cross-method checks with isapprox + an absolute tolerance; the dense and on-demand paths drift by a few ULPs run-to-run on the IS4/psy6 stack and many entries are physically zero (relative tolerance is meaningless there). - test_equivalent_getters.jl: attach the parallel branches to the system (skip_validation) so the system-base susceptance weighting can resolve. - test_modf_lodf_reductions.jl: clone transformers via deepcopy + fresh identity instead of re-reading each field, and pass PSY.SU to get_series_susceptance on attached branches. - test_ward_reduction.jl: read the detached ward-equivalent's susceptance with device base (DU). - test_ybus_reductions.jl: setters now require unitful values (val * PSY.SU). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
Thanks for opening a PR to PowerNetworkMatrices.jl, please take note of the following when making a PR:
Check the contributor guidelines
Summary
Updates PowerNetworkMatrices for the units-aware InfrastructureSystems / PowerSystems API (part of the ongoing units migration). PSY field getters now require an explicit unit system instead of relying on a mutable global unit setting.
Changes
Ybus,BA/ABA, branch reductions): impedance/admittance/shunt getters (get_r,get_x,get_b,get_g,get_primary_shunt, the 3W winding variants,get_impedance_active/reactive_power) now passSU(system base) — the basis the matrices are built in.BranchesParallel/BranchesSeries/ThreeWindingTransformerWinding): useDU(device base). These are metadata aggregations that must work on branches not yet attached to a system, and match the raw stored ratings.get_series_susceptance: threadedunits::IS.AbstractUnitSystemthrough the PNM wrappers (PSY's accessor now takes it), terminating atSUwhere the system-base matrices are assembled.[sources]: pinned IS/PSY (and test deps PSB/PFP) to their units-aware branches so CI can resolve the as-yet-unreleased upstream changes.maininto this branch and resolved conflicts insrc/BranchesParallel.jlandsrc/Ybus.jl, preserving the units-aware getter changes while incorporating upstream updates.Testing
Attempted
julia --project=. test/runtests.jlafter merge conflict resolution, but dependency resolution is currently blocked by a missing upstream revision (lk/units-fold-psu) inPowerSystems.jlin this environment.Dependencies
Depends on the units branches of InfrastructureSystems, PowerSystems, PowerSystemCaseBuilder, and a PowerFlowFileParser fix (separate PR). CI will be red until those merge/release; the
[sources]pins let it resolve against the branches meanwhile.🤖 Generated with Claude Code