Skip to content

implement native networkmodels#105

Open
jd-lara wants to merge 28 commits into
mainfrom
jd/implement_native_networkmodels
Open

implement native networkmodels#105
jd-lara wants to merge 28 commits into
mainfrom
jd/implement_native_networkmodels

Conversation

@jd-lara

@jd-lara jd-lara commented May 4, 2026

Copy link
Copy Markdown
Member

This is part 1 of removing the dependency on PowerModels. More testing ideas from what's here already are welcome.

Update: rebase onto main + PSY6/IOM-main units management

Per @jd-lara's request, the branch was updated to current main with the new dependencies and unit-system management:

  • Merged origin/main into the branch and resolved conflicts in src/ac_transmission_models/branch_constructor.jl (kept both the new Transformer3W native DCP/ACP construct_device! methods and the new TwoTerminalVSCLine methods from main) and test/Project.toml.
  • Pointed [sources] at the Sienna-Platform org with IS4, psy6, and main for InfrastructureOptimizationModels (plus psy6 for PowerNetworkMatrices/PowerSystemCaseBuilder and lk/psy6-units for PowerFlows in the test environment).
  • Renamed leftover OperationsProblemTemplate references in the native DCP/ACP construct_network! methods to PowerOperationsProblemTemplate.
  • Added the PSY.SU unit-system argument to PSY accessors used by the native branch_admittance and branch_flow_limits helpers (get_series_admittance, get_b, get_primary_shunt, get_r, get_x, get_flow_limits, get_rating) to align with PSY6 conventions.
  • Updated test/test_native_dcp_acp_models.jl to use the unit-system aware accessors.

test_native_dcp_acp_models and test_network_constructors pass under PSY6/IOM-main. Some unrelated pre-existing variable-bound accessors in src/ac_transmission_models/AC_branches.jl (the get_variable_upper_bound/get_variable_lower_bound definitions for MonitoredLine, TapTransformer, and Transformer2W) still call PSY.get_flow_limits(d)/PSY.get_rating(d) without PSY.SU; these surface in test_device_branch_constructors after the PSY6 bump but are outside the native NetworkModel scope of this PR.

# 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

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For situation like this use a network_common.jl file

@@ -0,0 +1,166 @@
# Native ACP network formulation. Provides bus voltage magnitude + angle

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check that this is tested with systems that contain multiple subnetworks with distinct slack buses

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +1212 to +1213
from_bus = PSY.get_number(PSY.get_from(PSY.get_arc(br)))
to_bus = PSY.get_number(PSY.get_to(PSY.get_arc(br)))
Comment on lines +1608 to +1614
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)
Comment on lines +1651 to +1667
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
Comment on lines +1727 to +1728
from_bus = PSY.get_name(from_bus_obj)
to_bus = PSY.get_name(to_bus_obj)
Comment on lines +1847 to +1848
from_bus = PSY.get_name(from_bus_obj)
to_bus = PSY.get_name(to_bus_obj)
Comment thread src/network_models/acp_model.jl Outdated

for k in subnet_keys
bus_set = subnets[k]
ref = _find_reference_bus(sys, bus_set)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should error earlier in the network model subnets. This code should be already covered in several of the network models checks.

Comment thread src/network_models/acp_model.jl Outdated
# 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

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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(

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This information is also available from the NetworkModel

@github-actions

github-actions Bot commented May 4, 2026

Copy link
Copy Markdown

Performance Results

Version Precompile Time
Main 2.427024899
This Branch 2.444172626
Version Build Time
Main-Build Time Precompile 67.434663518
Main-Build Time Postcompile 1.453816347
This Branch-Build Time Precompile 67.414148272
This Branch-Build Time Postcompile 1.700565711
Version Solve Time
Main-Solve Time Precompile FAILED TO TEST
Main-Solve Time Postcompile FAILED TO TEST
This Branch-Solve Time Precompile FAILED TO TEST
This Branch-Solve Time Postcompile FAILED TO TEST

@luke-kiernan luke-kiernan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Big picture things:

  1. In my opinion, JuMP calls 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.
  2. 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)
end

Is 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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_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!(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

@jd-lara

jd-lara commented May 5, 2026

Copy link
Copy Markdown
Member Author

@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

@jd-lara

jd-lara commented Jun 8, 2026

Copy link
Copy Markdown
Member Author

@copilot this PR needs to be updated to the new main branch with new dependencies and units management for the native network model implementation. Make the updates and make the tests work considering the sources IS4, PSY6 and IOM main

@jd-lara jd-lara requested a review from Copilot June 14, 2026 03:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 57 out of 57 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

test/test_utils/add_branch_rating_time_series.jl:34

  • scaling_factor_multiplier = get_rating is likely incompatible with PSY6’s unit-system-aware get_rating API (which typically requires PSY.SU/PSY.NU). Passing the bare function risks a MethodError when the multiplier is evaluated. Prefer an explicit closure that supplies the unit system.
    src/operation/emulation_model.jl:238
  • Renaming the keyword from export_optimization_model to export_optimization_problem is a breaking API change. Because this method still forwards kwargs... into other calls, the old keyword may now be silently forwarded and error downstream. Consider accepting export_optimization_model as a deprecated alias that overrides export_optimization_problem when provided.
function run!(
    model::EmulationModel{<:AbstractPowerEmulationProblem};
    export_problem_outputs = false,
    console_level = Logging.Error,
    file_level = Logging.Info,
    disable_timer_outputs = false,
    export_optimization_problem = true,
    enable_progress_bar = _progress_meter_enabled(),
    store_system_in_results = true,
    kwargs...,
)
    if store_system_in_results
        @warn "store_system_in_results is set to true. This will do nothing unless a Simulation is being built."
    end

Comment thread test/Project.toml
Comment on lines 35 to +41
[sources]
InfrastructureOptimizationModels = {path = "/Users/jlara/cache/PowerOperationsModels.jl/InfrastructureOptimizationModels.jl"}
InfrastructureSystems = {rev = "IS4", url = "https://github.com/Sienna-Platform/InfrastructureSystems.jl"}
PowerSystems = {rev = "psy6", url = "https://github.com/Sienna-Platform/PowerSystems.jl"}
InfrastructureOptimizationModels = {rev = "main", url = "https://github.com/Sienna-Platform/InfrastructureOptimizationModels.jl"}
PowerFlows = {rev = "psy6", url = "https://github.com/Sienna-Platform/PowerFlows.jl"}
PowerNetworkMatrices = {path = "/Users/jlara/cache/PowerOperationsModels.jl/PowerNetworkMatrices.jl"}
PowerSystemCaseBuilder = {rev = "psy6", url = "https://github.com/Sienna-Platform/PowerSystemCaseBuilder.jl"}
PowerNetworkMatrices = {rev = "psy6", url = "https://github.com/Sienna-Platform/PowerNetworkMatrices.jl"}
PowerFlows = {rev = "lk/psy6-units", url = "https://github.com/Sienna-Platform/PowerFlows.jl"}
PowerSystems = {rev = "psy6", url = "https://github.com/Sienna-Platform/PowerSystems.jl"}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants