Asymmetric line flow (Refined); READY FOR REVIEW !!! #789
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
lbonaldo
force-pushed
the
asymmetric_line_flow
branch
from
0002caf to
0e224c5
Compare
sambuddhac
changed the title
|
Hi @JesseJenkins and @lbonaldo , I have pushed all the modifications to the code base for this PR. Few things to note: First of all, as you will see, it's way more lengthy and complicated than what Jesse suggested to me previously. I first tried with the simpler version. But, was getting bidirectional flows on the asymmetric lines with Qian's example case even when we restricted the flow limits to zero in one of the directions for the asymmetric lines. With some pondering, I realized that I was doing wrong indexing on the constraints and decision variables. So, I had to do a bit more digging and refining the code. The fundamental reasons for which the code is so much lengthier are: 1) I have both symmetric as well as asymmetric lines in a system and 2) In order to maintain backward compatibility with the existing example cases (As you will observe, I had to go to lengths to split the data-frame into two parts and assign separate variables and constraint names so that things stay clean and correct. I could not find an easier way to do this. I'll highly appreciate any feedback on this). We tested with @qluo0320github 's example cases and it seems the results are sensible (?) However, I still get discrepancies when I benchmark against an actually symmetric system versus a "simulated" symmetric system (in which the asymmetric lines have the same flow limits, loss percentages, and transmission buildout in both directions). I am attaching hereby all the relevant flow comparison I scrutinized the code several times, but couldn't spot any flaw so far. I would appreciate if you could point some obvious bug (if at all) that I might have missed. Also, I would appreciate taking a look at the quadratic loss formulation. I believe I did it right, but am not totally certain. Thank you so much !!! I have also updated most of the doc pages; I will wrap up parts of the transmission.jl doc page and make another push to close this. |
sambuddhac
changed the title
There was a problem hiding this comment.
Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit
JuliaFormatter
[JuliaFormatter] reported by reviewdog 🐶
GenX.jl/src/model/core/transmission/transmission.jl
Lines 280 to 282 in 7d90bd9
[JuliaFormatter] reported by reviewdog 🐶
GenX.jl/src/model/core/transmission/transmission.jl
Lines 289 to 290 in 7d90bd9
[JuliaFormatter] reported by reviewdog 🐶
GenX.jl/src/model/core/transmission/transmission.jl
Lines 294 to 297 in 7d90bd9
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
GenX.jl/src/model/core/transmission/transmission.jl
Lines 470 to 471 in 7d90bd9
[JuliaFormatter] reported by reviewdog 🐶
GenX.jl/src/model/core/transmission/transmission.jl
Lines 473 to 474 in 7d90bd9
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
GenX.jl/src/model/core/transmission/transmission.jl
Lines 548 to 549 in 7d90bd9
[JuliaFormatter] reported by reviewdog 🐶
GenX.jl/src/model/core/transmission/transmission.jl
Lines 551 to 552 in 7d90bd9
[JuliaFormatter] reported by reviewdog 🐶
GenX.jl/src/model/core/transmission/transmission.jl
Lines 558 to 559 in 7d90bd9
[JuliaFormatter] reported by reviewdog 🐶
GenX.jl/src/model/core/transmission/transmission.jl
Lines 561 to 562 in 7d90bd9
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
GenX.jl/src/model/core/transmission/transmission.jl
Lines 625 to 626 in 7d90bd9
[JuliaFormatter] reported by reviewdog 🐶
GenX.jl/src/model/core/transmission/transmission.jl
Lines 628 to 629 in 7d90bd9
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
There was a problem hiding this comment.
Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit
JuliaFormatter
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
GenX.jl/src/model/core/transmission/transmission.jl
Lines 628 to 629 in 146916a
[JuliaFormatter] reported by reviewdog 🐶
GenX.jl/src/model/core/transmission/transmission.jl
Lines 631 to 632 in 146916a
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
[JuliaFormatter] reported by reviewdog 🐶
… asymmetrical flows
…olumns for bidirectional asymmetric flow
…refined write_expansion
Co-authored-by: Luca Bonaldo <[email protected]>
Deleted L_sym
Deleted unused L_asym, L_sym, and L
Corrected line numbering
Deleted unused EXPANSION_LINES and EXPANSION_LINES_ASYMMETRIC
…al flag is turned on but the corresponding columns are not provided in the Networks.csv
|
I believe "asymmetrical_trans_flow_limit" should be used in settings,
instead of AsymmetricalTransFlowLimit.
…On Thu, Feb 27, 2025, 11:37 Chakrabarti, Sambuddha (Sam) < ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/load_inputs/load_network_data.jl
<#789 (comment)>:
> + if setup["asymmetrical_trans_flow_limit"] == 1
+ L_asym = length(filtered_vector(network_var, :Asymmetrical, 1, :Network_Lines)) #Number of asymmetrical lines
+ end
@lbonaldo <https://github.com/lbonaldo> I fixed this with the latest push
and tested also. In order to review, for instance, you could take any of
the usual example cases (i.e. not the asymmetrical cases) and in the
settings file, put the AsymmetricalTransFlowLimit switch and set it to 0
—
Reply to this email directly, view it on GitHub
<#789 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALBIOTT77JINVQFFQJPLHND2R45M7AVCNFSM6AAAAABRXAC3KKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDMNBYGQYTEMBWGU>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
…xpansion lines in network load file to speed up computation
…onal flows and fixed the loss writing
|
@lbonaldo this is now fully ready (except that one tiny part where we might want to finalize the set of input example cases; what I can also do, is to change the Neg in all of them and re-number the cases, I can do that tomorrow). Also, I rebased with |
|
@lbonaldo , @qluo0320github , @JesseJenkins , done with everything on this PR. It should now be fully polished and ready to be merged. Please take a look and see if there's any more suggestions. |
…ample cases to test multistage asymmetric builds
Add warning for potential issues with asymmetric flow in multistage capacity expansion.
There was a problem hiding this comment.
Pull Request Overview
This PR introduces bidirectional asymmetric transmission lines into the GenX power system optimization model. The implementation allows transmission lines to have different flow limits, loss percentages, and capacity expansion limits in each direction, while maintaining backward compatibility with existing symmetric transmission lines.
Key changes:
- Added support for asymmetric transmission flow limits with separate positive and negative direction parameters
- Modified transmission modeling constraints and variables to handle both symmetric and asymmetric lines
- Updated data loading and output writing functions to support the new asymmetric line parameters
Reviewed Changes
Copilot reviewed 239 out of 300 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
src/model/core/transmission/transmission.jl |
Core transmission modeling with asymmetric flow limits and loss calculations |
src/model/core/transmission/investment_transmission.jl |
Investment planning for asymmetric transmission capacity expansion |
src/load_inputs/load_network_data.jl |
Data loading logic for asymmetric line parameters |
src/write_outputs/transmission/*.jl |
Output writing functions updated for asymmetric transmission results |
src/multi_stage/dual_dynamic_programming.jl |
Multi-stage modeling support for asymmetric transmission |
src/configure_settings/configure_settings.jl |
Added asymmetric transmission flow limit setting |
| Example systems and test cases | Multiple example cases demonstrating asymmetric transmission functionality |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| LOSS_LINES_ASYM = inputs["LOSS_LINES_ASYM"] # Lines for which loss coefficients apply (are non-zero); | ||
|
|
||
|
|
||
| LOSS_LINES_ASYM = inputs["LOSS_LINES_ASYM"] # Lines for which loss coefficients apply (are non-zero); |
There was a problem hiding this comment.
This line is a duplicate of line 8. The redundant assignment should be removed.
| LOSS_LINES_ASYM = inputs["LOSS_LINES_ASYM"] # Lines for which loss coefficients apply (are non-zero); |
Copilot uses AI. Check for mistakes.
| return models_d, stats_d, inputs_d | ||
| end | ||
|
|
||
| using Infiltrator |
There was a problem hiding this comment.
The using Infiltrator statement appears to be debugging code that should be removed from production code.
| using Infiltrator |
Copilot uses AI. Check for mistakes.
| # and the associated linking constraint name (c) as a value | ||
| for (e, c) in start_cap_d | ||
| for y in keys(EP_cur[c]) | ||
| @infiltrate # Use Infiltrator to inspect the keys of the constraint |
There was a problem hiding this comment.
This infiltrator debugging macro should be removed from production code.
| @infiltrate # Use Infiltrator to inspect the keys of the constraint |
Copilot uses AI. Check for mistakes.
| @infiltrate # Use Infiltrator to inspect the keys of the constraint | ||
| # Set the right hand side value of the linking initial capacity constraint in the current stage to the value of the available capacity variable solved for in the previous stages | ||
| if length(EP_prev[e]) != length(EP_cur[c]) # Check if the previous stage has a value for the variable | ||
| @error "The lengths of the dictionary EP_prev and EP_cur should be the same" # Skip if there is no value for the variable in the previous stage |
There was a problem hiding this comment.
The error message is unclear. It should specify which dictionaries are being compared and what the actual lengths are. Consider: @error \"Length mismatch between EP_prev[$(e)] ($(length(EP_prev[e]))) and EP_cur[$(c)] ($(length(EP_cur[c])))\"
| @error "The lengths of the dictionary EP_prev and EP_cur should be the same" # Skip if there is no value for the variable in the previous stage | |
| @error "Length mismatch between EP_prev[$(e)] ($(length(EP_prev[e]))) and EP_cur[$(c)] ($(length(EP_cur[c])))" |
Copilot uses AI. Check for mistakes.
| for i in eachindex(EXPANSION_LINES_ASYM) | ||
| asym_line_index = EXPANSION_LINES_ASYM[i] |
There was a problem hiding this comment.
The loop uses eachindex(EXPANSION_LINES_ASYM) but then accesses EXPANSION_LINES_ASYM[i] inside the loop. This is unnecessarily complex. Consider using for asym_line_index in EXPANSION_LINES_ASYM for clarity.
| for i in eachindex(EXPANSION_LINES_ASYM) | |
| asym_line_index = EXPANSION_LINES_ASYM[i] | |
| for (i, asym_line_index) in enumerate(EXPANSION_LINES_ASYM) |
Copilot uses AI. Check for mistakes.
| #@variable(EP, vTRANSMAX[l in SYMMETRIC_EXPANSION_LINES]>=0) | ||
| #@variable(EP, vTRANSMAX_Pos[l in EXPANSION_LINES_ASYM]>=0) | ||
| #@variable(EP, vTRANSMAX_Neg[l in EXPANSION_LINES_ASYM]>=0) |
There was a problem hiding this comment.
These commented-out variable declarations should be removed as they are not needed and create confusion.
| #@variable(EP, vTRANSMAX[l in SYMMETRIC_EXPANSION_LINES]>=0) | |
| #@variable(EP, vTRANSMAX_Pos[l in EXPANSION_LINES_ASYM]>=0) | |
| #@variable(EP, vTRANSMAX_Neg[l in EXPANSION_LINES_ASYM]>=0) |
Copilot uses AI. Check for mistakes.
| if c == :cExistingTransCap | ||
| set_normalized_rhs(EP_cur[c][y], value(EP_prev[e][y])) | ||
| elseif c == :cExistingTransCapPos | ||
| set_normalized_rhs(EP_cur[c][y], value(EP_prev[e][y])) | ||
| elseif c == :cExistingTransCapNeg | ||
| set_normalized_rhs(EP_cur[c][y], value(EP_prev[e][y])) |
There was a problem hiding this comment.
The three conditional branches have identical logic. This code duplication should be consolidated into a single conditional check for transmission capacity constraints.
| if c == :cExistingTransCap | |
| set_normalized_rhs(EP_cur[c][y], value(EP_prev[e][y])) | |
| elseif c == :cExistingTransCapPos | |
| set_normalized_rhs(EP_cur[c][y], value(EP_prev[e][y])) | |
| elseif c == :cExistingTransCapNeg | |
| set_normalized_rhs(EP_cur[c][y], value(EP_prev[e][y])) | |
| if c in (:cExistingTransCap, :cExistingTransCapPos, :cExistingTransCapNeg) | |
| set_normalized_rhs(EP_cur[c][y], value(EP_prev[e][y])) |
Copilot uses AI. Check for mistakes.
| cTAuxNeg2UB_asym[l in LOSS_LINES_ASYM, t = 1:T], | ||
| vTAUX_NEG_ASYM[l, t] <= vPROD_TRANSCAP_ON_NEG_ASYM[l, t] |
There was a problem hiding this comment.
These two constraints create a logical inconsistency. Constraint 306 requires vTAUX_NEG_ASYM[l, t] <= EP[:eAvail_Trans_Cap_Neg][l] - vPROD_TRANSCAP_ON_NEG_ASYM[l, t] while constraint 309 requires vTAUX_NEG_ASYM[l, t] <= vPROD_TRANSCAP_ON_NEG_ASYM[l, t]. Combined, these would force 2 * vPROD_TRANSCAP_ON_NEG_ASYM[l, t] >= EP[:eAvail_Trans_Cap_Neg][l], which may not be the intended behavior.
| cTAuxNeg2UB_asym[l in LOSS_LINES_ASYM, t = 1:T], | |
| vTAUX_NEG_ASYM[l, t] <= vPROD_TRANSCAP_ON_NEG_ASYM[l, t] | |
| # cTAuxNeg2UB_asym[l in LOSS_LINES_ASYM, t = 1:T], | |
| # vTAUX_NEG_ASYM[l, t] <= vPROD_TRANSCAP_ON_NEG_ASYM[l, t] |
Copilot uses AI. Check for mistakes.
5 participants
Description
This PR attempts to introduce bidirectional asymmetric transmission lines into the system. Such lines have different values of MW flow limits, loss percentage, and capacity expansion limits along the two directions. While doing so, this PR also ensures that there are also symmetric lines present in the system along with the asymmetric lines and modifies the code in such a way that backwards compatibility is maintained with the existing cases.
What type of PR is this? (check all applicable)
Related Tickets & Documents
Checklist
How this can be tested
Post-approval checklist for GenX core developers
After the PR is approved