Claude/inspiring johnson pqzkyz#1695
Merged
Merged
Conversation
…control traits for StaticInjection types Co-authored-by: jd-lara <16385323+jd-lara@users.noreply.github.com> Agent-Logs-Url: https://github.com/NREL-Sienna/PowerSystems.jl/sessions/b4e1573e-353e-4de2-a675-beeeb8ddbd2e
…ontrol_mode and bus type Co-authored-by: jd-lara <16385323+jd-lara@users.noreply.github.com> Agent-Logs-Url: https://github.com/NREL-Sienna/PowerSystems.jl/sessions/65d79b71-2f1c-4c06-b213-6b72d532bed5
…omment Co-authored-by: jd-lara <16385323+jd-lara@users.noreply.github.com> Agent-Logs-Url: https://github.com/NREL-Sienna/PowerSystems.jl/sessions/65d79b71-2f1c-4c06-b213-6b72d532bed5
…e for NML mode) Co-authored-by: m-bossart <67015312+m-bossart@users.noreply.github.com> Agent-Logs-Url: https://github.com/NREL-Sienna/PowerSystems.jl/sessions/4e8ef608-6efd-4060-b37e-d9bbcfceef46
…ride) Co-authored-by: jd-lara <16385323+jd-lara@users.noreply.github.com> Agent-Logs-Url: https://github.com/NREL-Sienna/PowerSystems.jl/sessions/48a5642f-e0b8-4fac-b6fa-a26153919ad0
…bus is never nothing) Co-authored-by: jd-lara <16385323+jd-lara@users.noreply.github.com> Agent-Logs-Url: https://github.com/NREL-Sienna/PowerSystems.jl/sessions/514ef393-9a13-4754-bbd1-ccff32acb69b
…-static-injectors
Minor Fix: docs badge link
…ons-build-job Fix broken SupplementalAttribute docs cross-reference
Deep multi-agent review of the IS4 (InfrastructureSystems) and psy6 units-management rework: 18 verified correctness findings, API hygiene, consolidation refactors, performance items, and a clean security pass. Written as a phased work order for a follow-up implementation session. https://claude.ai/code/session_013x42ojzmA6xjkgxQxriteV
…tatic-injectors Add traits to static injectors about active power, reactive power, voltage control
…son-pqzkyz # Conflicts: # test/test_devices.jl
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates PowerSystems.jl’s unit-conversion and accessor plumbing (explicit unit arguments, safer conversions for detached components, and improved quantity serialization), fixes/strengthens branch validation and logging, and re-enables a large set of integration tests that were previously commented out.
Changes:
- Harden unit conversion/serialization: base-voltage-required conversions now error clearly; DU↔SU conversions avoid unnecessary base-voltage access; RelativeQuantity serialization uses the unit type parameter.
- Improve robustness of display/printing and branch validation (including a fixed voltage-limit bug in
line_rating_calculationand clearer negative-rating error logs). - Re-enable substantial end-to-end test coverage (topology, services, serialization, hybrid system, bus numbering, printing) and expand unit/base-power regression tests.
Reviewed changes
Copilot reviewed 36 out of 77 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/test_units.jl | Adds unit-conversion regression tests, including base-voltage-missing error behavior. |
| test/test_topology.jl | Re-enables topology mapping and aggregation-topology tests. |
| test/test_subsystems.jl | Re-enables subsystem consistency tests, including hybrid subsystem behavior. |
| test/test_services.jl | Re-enables service removal and TransmissionInterface tests. |
| test/test_serialization.jl | Re-enables multiple serialization/deserialization testsets (RTS/matpower/hybrid/ext-fields/UUIDs). |
| test/test_printing.jl | Re-enables printing tests and adds detached-component REPL display regression coverage. |
| test/test_internal.jl | Re-enables internal UUID validation test. |
| test/test_hybrid_system.jl | Re-enables HybridSystem parsing/unattached-subcomponent test coverage. |
| test/test_busnumberchecks.jl | Re-enables matpower bus index/uniqueness validation tests. |
| test/test_branchchecks_testing.jl | Adds regression tests for negative ratings and voltage-limit selection in line rating calculation. |
| test/test_base_power.jl | Updates base-power tests and adds extensive transformer (2W/3W) unit conversion/setter round-trip tests. |
| test/Project.toml | Adds parser packages (currently under [extras]). |
| test/common.jl | Switches tests to use Unitful.ustrip directly. |
| src/utils/print.jl | Makes accessor printing more robust for detached components by falling back to DU when NU fails. |
| src/utils/IO/branchdata_checks.jl | Fixes line_rating_calculation to use the to-side Vmin; improves negative-rate logging; adds negative transformer rate guard. |
| src/units/types.jl | Updates UnitArg to IS.AbstractUnitSystem and adds Unitful.ustrip support for IS.RelativeQuantity. |
| src/units/serialization.jl | Removes unused StructTypes import; fixes RelativeQuantity unit serialization to derive unit from type parameter. |
| src/units/conversions.jl | Adds base-voltage checks; simplifies DU↔SU relative conversions via _du_to_su_ratio; removes bespoke multi-winding conversion family in favor of per-winding base providers. |
| src/PowerSystems.jl | Exports new supports_* helpers; removes exported ustrip; updates imports (adds get_value/set_value, uses Unitful.ustrip). |
| src/models/supplemental_accessors.jl | Refines “rounder” voltage selection logic; adds supports_active_power/supports_reactive_power/supports_voltage_control overrides for several device types. |
| src/models/static_models.jl | Introduces base supports_active_power / supports_reactive_power / supports_voltage_control defaults for StaticInjection. |
| src/models/generated/VariableReserveNonSpinning.jl | Updates get_requirement docs and display_units_arg formatting for explicit-units accessors. |
| src/models/generated/VariableReserve.jl | Makes requirement unit-aware via get_value/set_value; updates docs to specify SYSTEM_BASE p.u. |
| src/models/generated/TwoTerminalLCCLine.jl | Updates unit-aware accessor docstrings and display_units_arg formatting. |
| src/models/generated/TwoTerminalGenericHVDCLine.jl | Updates unit-aware accessor docstrings and display_units_arg formatting. |
| src/models/generated/TransmissionInterface.jl | Updates unit-aware accessor docstrings and display_units_arg formatting. |
| src/models/generated/Transformer2W.jl | Updates unit-aware accessor docstrings and display_units_arg formatting. |
| src/models/generated/TModelHVDCLine.jl | Updates unit-aware accessor docstrings and display_units_arg formatting. |
| src/models/generated/ThermalStandard.jl | Updates unit-aware accessor docstrings and display_units_arg formatting. |
| src/models/generated/ThermalMultiStart.jl | Updates unit-aware accessor docstrings and display_units_arg formatting. |
| src/models/generated/TapTransformer.jl | Updates unit-aware accessor docstrings and display_units_arg formatting. |
| src/models/generated/SynchronousCondenser.jl | Updates unit-aware accessor docstrings and display_units_arg formatting. |
| src/models/generated/Source.jl | Updates unit-aware accessor docstrings and display_units_arg formatting. |
| src/models/generated/ShiftablePowerLoad.jl | Updates unit-aware accessor docstrings and display_units_arg formatting. |
| src/models/generated/RenewableNonDispatch.jl | Updates unit-aware accessor docstrings and display_units_arg formatting. |
| src/models/generated/RenewableDispatch.jl | Updates unit-aware accessor docstrings and display_units_arg formatting. |
| src/models/generated/PowerLoad.jl | Updates unit-aware accessor docstrings and display_units_arg formatting. |
| src/models/generated/PhaseShiftingTransformer.jl | Updates unit-aware accessor docstrings and display_units_arg formatting. |
| src/models/generated/MotorLoad.jl | Updates unit-aware accessor docstrings and display_units_arg formatting. |
| src/models/generated/LoadZone.jl | Updates unit-aware accessor docstrings and display_units_arg formatting. |
| src/models/generated/Line.jl | Updates unit-aware accessor docstrings and display_units_arg formatting. |
| src/models/generated/InterruptiblePowerLoad.jl | Updates unit-aware accessor docstrings and display_units_arg formatting. |
| src/models/generated/InterconnectingConverter.jl | Updates unit-aware accessor docstrings and display_units_arg formatting. |
| src/models/generated/HydroTurbine.jl | Updates unit-aware accessor docstrings and display_units_arg formatting. |
| src/models/generated/HydroDispatch.jl | Updates unit-aware accessor docstrings and display_units_arg formatting. |
| src/models/generated/GenericArcImpedance.jl | Updates unit-aware accessor docstrings and display_units_arg formatting. |
| src/models/generated/ExponentialLoad.jl | Updates unit-aware accessor docstrings and display_units_arg formatting. |
| src/models/generated/EnergyReservoirStorage.jl | Updates unit-aware accessor docstrings and display_units_arg formatting. |
| src/models/generated/DiscreteControlledACBranch.jl | Updates unit-aware accessor docstrings and display_units_arg formatting. |
| src/models/generated/ConstantReserveNonSpinning.jl | Updates unit-aware accessor docstrings and display_units_arg formatting. |
| src/models/generated/ConstantReserveGroup.jl | Updates unit-aware accessor docstrings and display_units_arg signatures for parametric type. |
| src/models/generated/ConstantReserve.jl | Updates unit-aware accessor docstrings and display_units_arg signatures for parametric type. |
| src/models/generated/AreaInterchange.jl | Updates unit-aware accessor docstrings and display_units_arg formatting. |
| src/models/generated/Area.jl | Updates unit-aware accessor docstrings and display_units_arg formatting. |
| src/models/cost_functions/ImportExportCost.jl | Updates constructor to use kwarg path for Nothing-handling; handles nothing offsets in curve validation. |
| src/emissions_data.jl | Fixes doc reference to supplemental attributes documentation anchor. |
| src/descriptors/power_system_structs.json | Marks VariableReserve.requirement as needing conversion (:mva) and clarifies doc comment. |
| src/definitions.jl | Adds ZERO_ADMITTANCE_THRESHOLD for admittance capability detection. |
| src/base.jl | Updates unit-base docs to emphasize metadata-only behavior under explicit-unit accessors. |
| README.md | Updates documentation badge link to published docs site. |
| Project.toml | Removes unused StructTypes dependency and compat constraint. |
| docs/src/explanation/per_unit.md | Updates per-unit documentation for explicit-units (PSY6) programming model and migration guidance. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
20
to
26
| Unitful = "1986cc42-f94f-5a68-af5c-568840ba703d" | ||
|
|
||
| [extras] | ||
| PowerFlowFileParser = "bed98974-b02e-5e2f-9ee0-a103f5c450dd" | ||
| PowerTableDataParser = "2b750c0e-0bff-11f1-9200-1befd75df6be" | ||
|
|
||
| [sources] |
luke-kiernan
approved these changes
Jun 11, 2026
luke-kiernan
left a comment
Contributor
There was a problem hiding this comment.
I like the simplifications. I did some pretty thorough performance optimization stuff on the old version, though. So requests:
- Verify that the internal conversions get compiled away:
code_loweredonget_active_power(g, {SU/DU/NU})should showgetfieldplus straight float arithmetic. - Verify that
ustripis a no-op when types are inferred:ustrip(get_active_power_unitful(g, {SU/DU/NU}))compiles to the same thing as the non-unitful getter. - Performance check: should get similar results to here
- Type stability audit.
Comment on lines
+22
to
+25
| [extras] | ||
| PowerFlowFileParser = "bed98974-b02e-5e2f-9ee0-a103f5c450dd" | ||
| PowerTableDataParser = "2b750c0e-0bff-11f1-9200-1befd75df6be" | ||
|
|
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.
Relies on the companion IS branch Sienna-Platform/InfrastructureSystems.jl#585