-
Notifications
You must be signed in to change notification settings - Fork 122
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
DC OPF #543
DC OPF #543
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## develop #543 +/- ##
==========================================
+ Coverage 0.07% 0.30% +0.22%
==========================================
Files 112 112
Lines 3800 3945 +145
==========================================
+ Hits 3 12 +9
- Misses 3797 3933 +136 ☔ View full report in Codecov by Sentry. |
Thank you @abdelrahman-ayad !!! I just had a cursory glance. I will get back to you on this with more questions and change requests over the coming days. |
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.
A few notes without testing this on my own:
- Please add documentation to /docs and also to the relevant docstrings. Let us know if you need help with this. In particular please make sure to document the new DC_OPF setting and to document the added Network.csv columns.
- Why is there a new ISONE_Trizone_24hrs system? Is this necessary? It doesn't appear to have the DC OPF inputs. Similarly, why is the ISONE_Trizone_FullTimeseries case modified? Please make sure to only modify files that are relevant to this PR.
- Please remove the changes to the Project.toml and Genx.jl files. There is a forthcoming PR to develop that will make it easier to run with the Gurobi solver instead of having to change the GenX source code.
- Please remove the change to load_load_data.jl
- Should :Line_Impedance_ohms be :Line_Impedance_Ohms for consistency?
- Please remove the changes to the top of transmission.jl that are not relevant to this PR.
- Just curious, why are the angles in degrees and not radians? Is the IEEE test case data in degrees?
cc @cfe316
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'm a bit confused how load_load_data.jl
is in the changelog at all, as this file has been renamed to load_demand_data.jl
.
As a further note for the IEEE_9BUS example, you should strive to make the example case as simple as possible:
|
@cfe316 I believe it is because this PR is based on main and not the latest version of develop. @abdelrahman-ayad, please either merge in the latest develop or rebase this branch on the latest develop. It will probably be easiest to first remove the extraneous changes I mentioned, and then rebase or merge, to minimize merge conflicts. |
This PR splits the transmission into two functions separating out the calculation of flows via DC-OPF model and non-DC-OPF model and with associated documentations for both (work in progress)
To call the function for DC-OPF based calculation of line flows and non-DC-OPF based calculations separately (work in progress)
…ration, and separate dc-opf constraints
Documentation for DC-OPF
The branch is now rebased on develop. |
…s.jl to output radians not degrees
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.
All looks good to me now.
Removed the comment "#DEV NOTE: add DC power flow related parameter inputs in a subsequent commit" It seems that was an intermediate comment in the process of writing the DC-OPF code
Removed extra empty lines
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 code and the doc now looks good to me. I am in the process of testing with Sienna for some benchmarking. After this, I'll approve
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.
Code reviewed, tested, and approved
I made some further modifications to this PR recently !!! I introduced a new example case within the DCOPF example folder, that I called |
@gmantegna could you make sure to check once if your requested reviews have been addressed or not? To me, it seems they are. Please let e know if you want to add anything more here? Or, else. I'll close it and merge with |
…ONE_Singlezone 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.
Overall looks okay but just a few specific questions as noted.
Removed commented print statement
Addressed the specific comments from @gmantegna . I am marking the changes requested as addressed (by dismissing) so that I can merge with |
Added DC OPF method to calculate the power flows in all lines. Tested with the IEEE 9BUS test case.
genx_settings.yml
Network.csv
input file