Add tap controls; refactor AC_branches.jl by acostarelli · Pull Request #239 · Sienna-Platform/PowerOperationsModels.jl

acostarelli · 2026-08-15T03:20:24Z

Add support for tap control objectives, and large refactoring of AC_branches.jl

Copilot

Pull request overview

This PR refactors transformer tap handling by moving tap control behavior into the native branch construction path (gated by an enable_controls attribute and per-circuit TransformerControlObjective), while reorganizing/renaming related tests and tightening network-reduction “irreducible bus” logic for controlled devices and outages.

Changes:

Add tap-control variable/constraint plumbing to the native AC/DC branch models (including new voltage- and reactive-flow control constraints).
Restructure and expand transformer tap tests into separate “fixed tap” vs “controls” suites; remove legacy/disabled tap-control test files.
Refactor irreducible-bus selection to incorporate outage-monitored/outaged components and controllable transformers.

Reviewed changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
test/test_voltage_control_tap_models.jl	Removes legacy VoltageControlTap model tests.
test/test_transformer_fixed_tap.jl	Adds fixed (off-nominal) tap physics tests across native models.
test/test_transformer_controls.jl	Adds transformer control-objective tests (tap variables + control bands).
test/test_power_flow_in_the_loop.jl	Removes commented PhaseShiftingTransformer PFitL test block.
test/test_postcontingency_mixed_outage_axes.jl	Updates outage pinning test to call the new helper.
test/test_native_transformer_tap.jl	Removes older fixed-tap + coefficient ground-truth tests (now relocated).
test/test_native_tapcontrol.jl	Removes disabled TapControl formulation tests.
test/test_native_network_reductions.jl	Removes commented PhaseAngleControl and tap regulated-bus error test blocks.
test/test_native_lpacc_model.jl	Re-enables/updates LPACC shunt validation gate test.
test/test_native_dcp_acp_models.jl	Re-enables `use_slacks` validation test for StaticBranchUnbounded.
test/test_device_branch_constructors.jl	Removes commented phase-shifting DC power flow test block.
test/runtests.jl	Removes DISABLED_TESTS entries (now empty).
test/Project.toml	Removes PowerFlows from test deps/sources (but tests still reference it).
test/includes.jl	Comments out `using PowerFlows` and `const PFS = PowerFlows` (but tests still reference `PFS`).
src/PowerOperationsModels.jl	Exports `ReactivePowerFlowControlConstraint`.
src/network_models/instantiate_network_model.jl	Refactors irreducible-bus selection; adds outage + controllable-transformer pinning.
src/core/network_formulations.jl	Removes old regulated-voltage/tap-current-form traits.
src/core/constraints.jl	Adds `ReactivePowerFlowControlConstraint` type.
src/ac_transmission_models/voltage_control_tap_models.jl	Removes legacy VoltageControlTap formulation implementation.
src/ac_transmission_models/branch_constructor.jl	Wires new tap-control variables/constraints into branch construction stages.
src/ac_transmission_models/AC_branches.jl	Implements tap-control variables, voltage/reactive control constraints, and tap-aware flow equations.

Suppressed comments (3)

test/includes.jl:39

PFS is still referenced by active tests (e.g. test/test_power_flow_in_the_loop.jl), but the alias is commented out. This will raise UndefVarError: PFS not defined at runtime.

const PSY = PowerSystems
const POM = PowerOperationsModels
const IOM = InfrastructureOptimizationModels
#const PFS = PowerFlows
const PSB = PowerSystemCaseBuilder
const PNM = PowerNetworkMatrices

test/Project.toml:23

PowerFlows is removed from [deps], but multiple active tests (not just commented blocks) still rely on it via the PFS alias (see test/test_power_flow_in_the_loop.jl). This will fail to instantiate the test environment.

This issue also appears on line 34 of the same file.

MathOptInterface = "b8f27783-ece8-5eb3-8dc8-9495eed66fee"
ParallelTestRunner = "d3525ed8-44d0-4b2c-a655-542cee43accc"
Pkg = "44cfe95a-1eb2-52ea-b672-e2afdf69b78f"
PowerNetworkMatrices = "bed98974-b02a-5e2f-9fe0-a103f5c450dd"
PowerOperationsModels = "bed98974-b02a-5e2f-9ee0-a103f5c450dd"
PowerSystemCaseBuilder = "f00506e0-b84f-492a-93c2-c0a9afc4364e"
PowerSystems = "bcd98974-b02a-5e2f-9ee0-a103f5c450dd"

test/Project.toml:40

PowerFlows is removed from [sources], but it’s still required by the active PFitL tests. Without a source entry (when using custom git sources for the other Sienna packages), resolving the test environment may pull an incompatible version or fail outright.

[sources]
InfrastructureOptimizationModels = {rev = "main", url = "https://github.com/Sienna-Platform/InfrastructureOptimizationModels.jl"}
InfrastructureSystems = {rev = "IS4", url = "https://github.com/Sienna-Platform/InfrastructureSystems.jl"}
PowerNetworkMatrices = {rev = "psy6", url = "https://github.com/Sienna-Platform/PowerNetworkMatrices.jl"}
PowerSystemCaseBuilder = {rev = "psy6", url = "https://github.com/Sienna-Platform/PowerSystemCaseBuilder.jl"}
PowerSystems = {rev = "psy6", url = "https://github.com/Sienna-Platform/PowerSystems.jl"}

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

acostarelli · 2026-08-15T20:14:23Z

 function _validate_controlled_branch_not_reduced(
    network_model::NetworkModel,
-    devices::IS.FlattenIteratorWrapper{T},
-    formulation_name::String,
+    ::Type{T},
+    controlled_names,
 ) where {T <: PSY.ACTransmission}


transformer_models.jl isn't used. I will remove it when I implement phase shifting.

I believe a parallel merge is the only merge that can happen; series merges are blocked by specifying irreducible buses.

acostarelli · 2026-08-15T20:12:58Z

+            for circuit in PSY.get_circuits(transformer)
+                _control_enabled(circuit) || continue
+                _push_component_buses!(irreducible_buses, circuit)
+                push!(irreducible_buses, PSY.get_regulated_bus_number(circuit))
+            end


I think the convention we're going to go for now is the regulated bus number must always be a valid bus number, so 0 doesn't carry special importance.

That said, I agree maybe we shouldn't be pinning the regulated bus on all controls.

jd-lara · 2026-08-15T23:30:00Z

@acostarelli I stacked this on top of the PR with the network changes

acostarelli

Implementation review

acostarelli

Tests review

Have not checked that old tap tests that should be ported were ported.

acostarelli

Test port check. Seems fine.

…ible

…tage and q bounds

fold branch_rate_bounds! into _branch_variable_bounds dispatches run during add_variables!, cleaner because variable bounds aren't changed later get_min_max and min_max_flow_limits -> _flow_limits(rep), symmetric line rating or possibly-asymmetric monitored line limits. reactive control only controls from_bus flow, I believe. delete transformer_models.jl better control validation: warn if a control is unsupported, treat as if uncontrolled for reductions. additionally compare regulated bus against VSCs in template validation

own VoltageControlConstraint now

fixed slack variable bounds added types to helpers because i keep passing in types that i dont want to accept and getting silent errors check for parallel reduction before checking if branch uses the control variable, because it obviously doesn't use the control if it's a reduction. add current control bounds check check for constraints in tests test no tap on dc add support for StaticBranchBounds, and coverage test _tapped_admittance against DC PF

my _branch_variable_bounds dispatch structure is a bit confusing, probably should be revised, but for now this fixes the infeasibilites.

acostarelli · 2026-08-21T05:47:13Z

@m-bossart I addressed your reviews. I have some responses to the one from Slack:
7. Should reactive power control only constrain the from-to flow? This is what I changed it to. Previously it was constraining both.
9. I added validation that the regulated bus number is valid, but I think this should happen in PSY
12. I'm choosing not to use regulated_voltage.jl because it feels quite overkill and my solution is cleaner IMO.
13. Rather than restoring the test with "hand-computed" susceptances, I have the test compare POM's calculated admittance against a DC PF, to ensure tap is included correctly

codecov · 2026-08-21T06:23:40Z

Codecov Report

❌ Patch coverage is 89.71963% with 22 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/ac_transmission_models/RepresentativeBranch.jl	89.07%	13 Missing ⚠️
src/network_models/reduction_exceptions.jl	84.84%	5 Missing ⚠️
src/ac_transmission_models/branch_constructor.jl	95.23%	2 Missing ⚠️
...transmission_models/security_constrained_branch.jl	66.66%	1 Missing ⚠️
src/operation/template_validation.jl	94.11%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

m-bossart

A few general comments.

The rating_b thing is the most concerning, I think that is a real regression that is made in this PR. It might be that we need better test coverage there.
There is still a bunch of commented/stale code that is no longer needed after this refactor. Search for "psy6: disabled pending transformer refactor". We can leave some of the code related to phase shifting for now, but some of that can be cleaned up.
Update the documentation on the transformer formulations. All of the information there is now stale.
TapRatioVariable's exported docstring
(core/variables.jl:65-66) reads "for a the disabled VoltageControlTap formulation". It references a deleted type on the public docstring.
I think it is OK to not reuse regulated_voltage.jl because we are implementing a voltage band and not fixing a specific value. But we should do some cleanup to that file to remove the functionality that was originally included for transformers (including the documentation).

m-bossart · 2026-08-21T17:07:07Z

+    _branch_rating(rep.branch, model)
+
+# TODO: SC branches don't use the RepBranch API but they need rating_b, so
+# move this or change SC to use this API.


_branch_rating_b is dispatching to just get_rating. This will return the normal rating even when there is an emergency rating defined. Something wrong here.

But yes coverage is not great

m-bossart · 2026-08-21T17:07:36Z

+_branch_rating(rep::RepresentativeBranch, model::DeviceModel) =
+    _branch_rating(rep.branch, model)
+
+# TODO: SC branches don't use the RepBranch API but they need rating_b, so


Can we address this now and not leave a TODO? What are the limitations to using RepresentativeBranch with SC models?

Seems like that whole _check_time_series method in add_parameters.jl isn't covered because it doesn't even have a correct _branch_rating signature.

m-bossart · 2026-08-21T17:54:48Z

@m-bossart I addressed your reviews. I have some responses to the one from Slack: 7. Should reactive power control only constrain the from-to flow? This is what I changed it to. Previously it was constraining both. 9. I added validation that the regulated bus number is valid, but I think this should happen in PSY 12. I'm choosing not to use regulated_voltage.jl because it feels quite overkill and my solution is cleaner IMO. 13. Rather than restoring the test with "hand-computed" susceptances, I have the test compare POM's calculated admittance against a DC PF, to ensure tap is included correctly

7., 12., 13. Ok - seems reasonable.
9. Yes, we need to make the changes in the parser/PSY/schemas to change the interpretation of the regulated bus numbers.

m-bossart · 2026-08-23T20:44:10Z

@jd-lara Only the pre-existing test failures are failing locally (services related)

acostarelli requested review from jd-lara and m-bossart and a lite review from Copilot August 15, 2026 03:20

Copilot started reviewing on behalf of acostarelli August 15, 2026 03:20 View session

Copilot AI reviewed Aug 15, 2026

View reviewed changes

jd-lara changed the base branch from main to jd/network_matrix_consolidation August 15, 2026 23:29

jd-lara force-pushed the jd/network_matrix_consolidation branch from ce24265 to 7ebf176 Compare August 16, 2026 16:22

acostarelli commented Aug 17, 2026

View reviewed changes

Anthony Costarelli added 9 commits August 17, 2026 13:59

refactor voltage-only ac network flow code

027f186

refactor irreducible bus code; make controllable transformers irreduc…

ce8924d

…ible

rough tap control implementation; still need to run tests and add vol…

ad78ddf

…tage and q bounds

finished tap controls

be410db

claude reorganize tests and update tap tests

702114c

claude ported tests; tests now pass

1cd82e6

claude finished the impl

97785c1

remove unnecessary test

e11cd7b

formatting

7368dcd

acostarelli force-pushed the ac/transformer-control branch from dd4ad35 to 7368dcd Compare August 17, 2026 18:27

Anthony Costarelli added 2 commits August 17, 2026 14:46

readded bus reduction protection; again removed pfs for now

fc4726f

address review comments; refactor IVR, rename helpers

abc4c2c

acostarelli commented Aug 17, 2026

View reviewed changes

Comment thread test/test_transformer_controls.jl Outdated

Base automatically changed from jd/network_matrix_consolidation to main August 18, 2026 03:43

Anthony Costarelli added 5 commits August 18, 2026 10:24

rest of review comments

69ce8fe

claude refactor add_variables!

7ea2ee9

claude refactor to RepresentativeBranch

8084026

cleaned up claude's code; control constraints use new api

ad042e9

tests pass

7d45fb4

Anthony Costarelli added 7 commits August 20, 2026 13:52

reenable pfs

92693ea

disable DC var tap control; warn non-support or not implemented

1847dc0

tests pass

94f4c54

own VoltageControlConstraint now

formatting

9218d93

fix infeasibility; all tests passing

1d74849

my _branch_variable_bounds dispatch structure is a bit confusing, probably should be revised, but for now this fixes the infeasibilites.

m-bossart reviewed Aug 21, 2026

View reviewed changes

Anthony Costarelli and others added 13 commits August 21, 2026 14:18

claude: sc uses rep branch

a78bf6a

matt review

29dee77

formatting

659e0ae

restore and test post contingency + time series rating

3f67b44

update formulation library

35c1130

small docstring fix

807bf04

fix and test warning of transformers with control

d297a6f

cleanup test helpers

7a77a99

only ac support taps in reductions

c38edfe

unused constant, extend to PTDF

2eb0970

remove stale code and incorrect validation

6a165f7

Merge branch 'main' into ac/transformer-control

b5098b7

typo from merge

9f4b1c0

m-bossart self-requested a review August 23, 2026 20:42

m-bossart approved these changes Aug 23, 2026

View reviewed changes

jd-lara approved these changes Aug 24, 2026

View reviewed changes

jd-lara merged commit eb9562f into main Aug 24, 2026
2 of 6 checks passed

rodrigomha added a commit that referenced this pull request Aug 24, 2026

Merge main (transformer-control #239) into rh/fix_services_new_tests

dd98c20

Uh oh!

Conversation

acostarelli commented Aug 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

acostarelli Aug 15, 2026

Choose a reason for hiding this comment

Uh oh!

acostarelli Aug 15, 2026

Choose a reason for hiding this comment

Uh oh!

jd-lara commented Aug 15, 2026

Uh oh!

acostarelli left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

acostarelli left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

acostarelli left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

acostarelli commented Aug 21, 2026

Uh oh!

codecov Bot commented Aug 21, 2026

Codecov Report

Uh oh!

m-bossart left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

m-bossart Aug 21, 2026

Choose a reason for hiding this comment

Uh oh!

acostarelli Aug 21, 2026

Choose a reason for hiding this comment

Uh oh!

acostarelli Aug 21, 2026

Choose a reason for hiding this comment

Uh oh!

m-bossart Aug 21, 2026

Choose a reason for hiding this comment

Uh oh!

acostarelli commented Aug 15, 2026 •

edited

Loading