Conversation
| # by the branch device construction path (Phase F). | ||
| # | ||
| # Convention: variables/constraints indexed by ACBus use bus NAME (String) — see dcp_model.jl. | ||
| # add_variables!(VoltageAngle, ...) for ACPPowerModel is defined in dcp_model.jl |
There was a problem hiding this comment.
For situation like this use a network_common.jl file
| @@ -0,0 +1,166 @@ | |||
| # Native ACP network formulation. Provides bus voltage magnitude + angle | |||
There was a problem hiding this comment.
Check that this is tested with systems that contain multiple subnetworks with distinct slack buses
There was a problem hiding this comment.
Pull request overview
This PR introduces native DCPPowerModel and ACPPowerModel network formulations inside PowerOperationsModels as a first step toward removing the dependency on the PowerModels bridge.
Changes:
- Added native DCP/ACP network model types and concrete
construct_network!implementations (voltage variables, reference-bus pinning, nodal balances). - Implemented native AC-branch primitives (admittance, flow limits) and native DCP/ACP branch construction paths (variables, Ohm’s law, rate limits, angle-difference limits, 3-winding transformer handling).
- Added expression wiring for native ACP directional branch flows and added initial unit tests for new branch primitives.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
test/test_native_dcp_acp_models.jl |
Adds unit tests for new branch_admittance and branch_flow_limits primitives. |
test/Project.toml |
Updates test dependency sources (notably points IOM to main). |
src/PowerOperationsModels.jl |
Stops importing PM’s DCPPowerModel/ACPPowerModel, includes native model files, exports new constraint types. |
src/network_models/network_constructor.jl |
Adds native construct_network! dispatch for NetworkModel{DCPPowerModel} and NetworkModel{ACPPowerModel}. |
src/network_models/dcp_model.jl |
Implements native DCP voltage-angle variables, reference bus pinning, and active nodal balance constraints. |
src/network_models/acp_model.jl |
Implements native ACP voltage-magnitude variables, reference bus pinning (vm/va), and active/reactive nodal balances. |
src/core/network_formulations.jl |
Defines native DCPPowerModel and ACPPowerModel marker structs. |
src/core/constraints.jl |
Introduces ReferenceBusConstraint and AngleDifferenceConstraint types. |
src/common_models/add_to_expression.jl |
Adds native ACP/DCP HVDC and ACP directional AC-branch flow contributions to nodal balances. |
src/ac_transmission_models/branch_constructor.jl |
Adds native DCP/ACP branch constructor dispatches, HVDC var creation, and Transformer3W-specific dispatches. |
src/ac_transmission_models/AC_branches.jl |
Adds primitives (branch_admittance, branch_flow_limits) and native DCP/ACP branch constraints (rate limits, Ohm’s law, angle limits, Transformer3W decomposition). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| from_bus = PSY.get_number(PSY.get_from(PSY.get_arc(br))) | ||
| to_bus = PSY.get_number(PSY.get_to(PSY.get_arc(br))) |
| for d in devices | ||
| name = PSY.get_name(d) | ||
| adm = branch_admittance(d) | ||
| from_bus_obj = PSY.get_from(PSY.get_arc(d)) | ||
| to_bus_obj = PSY.get_to(PSY.get_arc(d)) | ||
| from_bus = PSY.get_name(from_bus_obj) | ||
| to_bus = PSY.get_name(to_bus_obj) |
| time_steps = get_time_steps(container) | ||
| network_reduction = get_network_reduction(network_model) | ||
| va = get_variable(container, VoltageAngle, PSY.ACBus) | ||
|
|
||
| branch_names = [PSY.get_name(d) for d in branches_with_limits] | ||
| cons = add_constraints_container!( | ||
| container, AngleDifferenceConstraint, T, branch_names, time_steps, | ||
| ) | ||
|
|
||
| for d in branches_with_limits | ||
| name = PSY.get_name(d) | ||
| lims = PSY.get_angle_limits(d) | ||
| from_bus_obj = PSY.get_from(PSY.get_arc(d)) | ||
| to_bus_obj = PSY.get_to(PSY.get_arc(d)) | ||
| from_bus = PSY.get_name(from_bus_obj) | ||
| to_bus = PSY.get_name(to_bus_obj) | ||
| for t in time_steps |
| from_bus = PSY.get_name(from_bus_obj) | ||
| to_bus = PSY.get_name(to_bus_obj) |
| from_bus = PSY.get_name(from_bus_obj) | ||
| to_bus = PSY.get_name(to_bus_obj) |
|
|
||
| for k in subnet_keys | ||
| bus_set = subnets[k] | ||
| ref = _find_reference_bus(sys, bus_set) |
There was a problem hiding this comment.
This should error earlier in the network model subnets. This code should be already covered in several of the network models checks.
| # Skip subnetworks without an AC reference bus (e.g. an isolated DC-side | ||
| # island connected through HVDC converters). | ||
| ref === nothing && continue | ||
| ref_name = ref.name |
There was a problem hiding this comment.
using dot access is a clear break from the Sienna.md practices. Unacceptable
| # System-balance expressions (ActivePowerBalance) are indexed by bus NUMBER | ||
| # (Int) per make_system_expressions.jl, so internal lookups translate names↔numbers. | ||
|
|
||
| function _bus_name_number_pairs( |
There was a problem hiding this comment.
This information is also available from the NetworkModel
|
Performance Results
|
luke-kiernan
left a comment
There was a problem hiding this comment.
Big picture things:
- In my opinion,
JuMPcalls only belong in POM if they're device-specific or otherwise bespoke. If you're just assigning upper/lower bounds with some slacks, I'd prefer to have the math live back in IOM. - In
construct_device!calls, we almost always write
add_variables!(container, FlowActivePowerFromToVariable, devices, StaticBranchBounds)
add_variables!(container, FlowActivePowerToFromVariable, devices, StaticBranchBounds)
add_variables!(container, FlowReactivePowerFromToVariable, devices, StaticBranchBounds)
add_variables!(container, FlowReactivePowerToFromVariable, devices, StaticBranchBounds)instead of
for flow_var in (FlowActivePowerFromToVariable, ...) # list 4 variable types
add_variables!(container, flow_var, devices, StaticBranchBounds)
endIs there a good reason for this pattern? I find the 2nd much easier to parse at a glance
| - `tap`: voltage-ratio magnitude (1.0 if not a tap-changing transformer) | ||
| - `shift`: nominal phase-shift angle in radians (0.0 if not a PST; the value of α for fixed PSTs) | ||
| """ | ||
| function branch_admittance end |
There was a problem hiding this comment.
Does branch_admittance really belong here in POM? I'd sooner put it in PNM or perhaps even in PSY for the raw components.
| Constrains pft^2 + qft^2 ≤ rating^2. Does not depend on PTDF / network-reduction | ||
| infrastructure; iterates directly over devices. | ||
| """ | ||
| function add_constraints!( |
There was a problem hiding this comment.
These next 2 functions only differ by FromTo versus ToFrom. Combine: either multiple dispatch pattern or just separate out the 3 get_variable calls and delegate the last dozen lines to a helper.
| This is a simple lb/ub pair on the FlowActivePowerVariable that does not depend on the | ||
| PTDF / network-reduction infrastructure used by the AbstractActivePowerModel dispatch. | ||
| """ | ||
| function add_constraints!( |
There was a problem hiding this comment.
There's nothing novel about the types or math here: imo this ought to be calling some IOM function. In the no-slacks case, add_range_constraints! works; we might need to add a variant of add_range_constraints! that allows for slacks.
| (i.e. not the ±π defaults) receive a constraint. Branches where the method is not defined | ||
| are silently skipped. | ||
| """ | ||
| function add_constraints!( |
There was a problem hiding this comment.
Looks identical or nearly identical to the DCPPowerModel implementation. Combine.
| Returns a tuple of 3 NamedTuples, one per winding of a Transformer3W: | ||
| (suffix, arc, r, x, rating, tap, base_power) | ||
| """ | ||
| function _three_winding_arcs(t::PSY.Transformer3W) |
There was a problem hiding this comment.
_three_winding_arcs and _winding_admittance seem like they belong in PNM or PSY.
|
|
||
| # Shared between DCPPowerModel and ACPPowerModel — both put VoltageAngle on every | ||
| # bus axis with no bounds. Slack-bus pinning is applied by ReferenceBusConstraint. | ||
| function add_variables!( |
There was a problem hiding this comment.
Not needed: looks equivalent to the generic add_variables! implementation in IOM.
edit: might need to define get_variable_binary to be false for that implementation to work.
| return nothing | ||
| end | ||
|
|
||
| function add_constraints!( |
There was a problem hiding this comment.
Same as above: check IOM for relevant helper functions
| to_bus = PSY.get_name(to_bus_obj) | ||
| # Pre-compute constant coefficients from tap + shift. | ||
| # Convention (same as PowerModels.jl ACP): | ||
| # tr = tm * cos(shift), ti = tm * sin(shift) |
There was a problem hiding this comment.
Yikes this is complicated. Would be good to get some tests here. Is there a simple way to tell if it's correct via complex number math or numerical approximations?
|
@luke-kiernan after a second look this PR needs more work since it isn't leveraging correctly the mappings in NetworkModels. I need to direct Claude better |
This is part 1 of removing the dependency on PowerModels. More testing ideas from what's here already are welcome