-
Notifications
You must be signed in to change notification settings - Fork 142
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add linear branch flow model and branch flow storage problem type #676
Conversation
…for AbstractWRModel
Codecov Report
@@ Coverage Diff @@
## master #676 +/- ##
==========================================
+ Coverage 92.94% 92.98% +0.04%
==========================================
Files 42 42
Lines 7014 7070 +56
==========================================
+ Hits 6519 6574 +55
- Misses 495 496 +1 Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution! Overall looks very good. I have a few minor requests about naming and best use of the type hierarchy.
Also, please add a note to the change log and add yourself to the list of contributors in the readme.
On a side note, can you tell me a little bit about your interest in this model? Do you have a specific application in mind or do you just like to have it for posterity sake?
src/core/types.jl
Outdated
@@ -28,6 +28,10 @@ abstract type AbstractConicModel <: AbstractPowerModel end | |||
"for branch flow models" | |||
abstract type AbstractBFModel <: AbstractPowerModel end | |||
|
|||
|
|||
"for variants of branch flow models that target LP solvers" | |||
abstract type AbstractBFLPModel <: AbstractBFModel end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the name AbstractBFAModel
for Branch Flow Approximation, would be more consistent with the other model naming conventions.
src/core/types.jl
Outdated
} | ||
``` | ||
""" | ||
mutable struct LPBFPowerModel <: AbstractBFLPModel @pm_fields end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The model name should match the abstract type name, so BFAPowerModel
following the previous comment.
src/form/shared.jl
Outdated
@@ -343,3 +343,28 @@ function constraint_current_limit(pm::AbstractWModels, n::Int, f_idx, c_rating_a | |||
JuMP.@constraint(pm.model, p_to^2 + q_to^2 <= w_to*c_rating_a^2) | |||
end | |||
|
|||
"" | |||
function constraint_storage_loss(pm::AbstractWModels, n::Int, i, bus, r, x, p_loss, q_loss; conductors=[1]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the more general type we want here is AbstractWConvexModels
. Note that AbstractWModels
includes AbstractACTModel
which is a nonconvex model that does not require a current variable. This constraint can be limited similarly to the ACPPowerModel
case.
src/form/wr.jl
Outdated
@@ -337,7 +337,7 @@ end | |||
|
|||
|
|||
"do nothing by default but some formulations require this" | |||
function variable_current_storage(pm::AbstractWRModel; nw::Int=pm.cnw, bounded::Bool=true, report::Bool=true) | |||
function variable_current_storage(pm::AbstractWModels; nw::Int=pm.cnw, bounded::Bool=true, report::Bool=true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above, AbstractWConvexModels
. Also now as a shared variable definition, it can move to form/shared.jl
.
test/opf.jl
Outdated
@test isapprox(result["objective"], 11277.9; atol = 1e0) | ||
end | ||
# @testset "24-bus rts case" begin | ||
# result = run_opf_bf("../test/data/matpower/case24.m", LPBFPowerModel, ipopt_solver) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious, what is the issue with this case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This case is infeasible for this formulations. I haven't figured out why.
The intention with the linear branch flow approximation is to use it for operation and sizing of storages in LV/MV grids. I need a computationally efficient implementation for problems with many timesteps. All your comments have been addressed and the code should be ready for a new review. Thanks for your feedback! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks for the contribution. Will merge once CI passes.
Would you like a new version of PowerModels tagged right away or can I wait until a few more updates have accumulated?
BTW, if you continue to work on your fork you will want to sync and start from the master as I squash merged this PR. |
Thanks! No need for tagging it right away, you can do it when you find it convenient. |
This PR adds the linearized branch flow model formulation for OPF and PF including tests. A storage problem for branch flow has also been added (
build_mn_opf_bf_strg
) and tested for both the new linearized branch flow and existing branch flow.build_mn_opf_bf_strg
is the branch flow equivalent of bus injection model problembuild_mn_opf_strg