Skip to content
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

Merged
merged 34 commits into from
Feb 26, 2024
Merged

DC OPF #543

merged 34 commits into from
Feb 26, 2024

Conversation

abdelrahman-ayad
Copy link
Contributor

Added DC OPF method to calculate the power flows in all lines. Tested with the IEEE 9BUS test case.

  • DC_OPF flag is added to genx_settings.yml
  • Lines reactances, bus voltage angle limits, and voltage levels to be defined in the Network.csv input file

@codecov-commenter
Copy link

codecov-commenter commented Sep 7, 2023

Codecov Report

Attention: 22 lines in your changes are missing coverage. Please review.

Comparison is base (951179b) 0.07% compared to head (0cb288e) 0.30%.
Report is 53 commits behind head on develop.

❗ Current head 0cb288e differs from pull request most recent head 3b22fe6. Consider uploading reports for the commit 3b22fe6 to get more accurate results

Files Patch % Lines
src/model/core/transmission.jl 0.00% 14 Missing ⚠️
src/load_inputs/load_network_data.jl 0.00% 6 Missing ⚠️
src/load_inputs/load_load_data.jl 0.00% 1 Missing ⚠️
src/write_outputs/write_costs.jl 0.00% 1 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

@sambuddhac
Copy link
Collaborator

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.

@sambuddhac sambuddhac changed the base branch from main to develop September 7, 2023 05:29
Copy link
Collaborator

@gmantegna gmantegna left a 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

Copy link
Collaborator

@cfe316 cfe316 left a 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.

@cfe316
Copy link
Collaborator

cfe316 commented Sep 28, 2023

As a further note for the IEEE_9BUS example, you should strive to make the example case as simple as possible:

  • time_domain_reduction_settings.yml can be removed as iirc the case in only a single hour
  • cbc and clp settings files can be removed as these are not in widespread use
  • Load_data.csv should be renamed to Demand_data.csv

@gmantegna
Copy link
Collaborator

@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.

@lbonaldo
Copy link
Collaborator

lbonaldo commented Feb 3, 2024

The branch is now rebased on develop.

@GenXProject GenXProject added this to the v0.4 milestone Feb 4, 2024
Copy link
Collaborator

@filippopecci filippopecci left a 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
Copy link
Collaborator

@sambuddhac sambuddhac left a 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

Copy link
Collaborator

@sambuddhac sambuddhac left a 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

@sambuddhac
Copy link
Collaborator

I made some further modifications to this PR recently !!! I introduced a new example case within the DCOPF example folder, that I called IEEE_9BUS_timeseries. It's essentially a modification in which I considered 8760 hour time series for the pertinent CSV files (with some reasonable manipulations). I also corrected the Fuels_data.csv (introduced time series and inserted sensible realistic values for the emissions). I also introduced unequal number of lines (versus nodes/buses/zones) to test the generality of the code. I think all of @gmantegna 's suggestions and questions have been addressed now. I also resolved the conflicts. The only two things I will now work on, before merging with develop is to get rid of the annual sum columns for writing bus voltage phase angles and line flows, because I don't thing that carries any meaning (and not write those when the write options is set to annual instead of full; correct me if I am wrong @JesseJenkins @filippopecci @dharik13 ) and fix the automated tests (since all of them are failing now)

@sambuddhac
Copy link
Collaborator

@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 develop. Thanks !!!

Copy link
Collaborator

@gmantegna gmantegna left a 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.

src/load_inputs/load_resources_data.jl Outdated Show resolved Hide resolved
src/model/core/fuel.jl Show resolved Hide resolved
src/model/core/transmission/transmission.jl Show resolved Hide resolved
Removed commented print statement
@sambuddhac
Copy link
Collaborator

Addressed the specific comments from @gmantegna . I am marking the changes requested as addressed (by dismissing) so that I can merge with develop. Thanks yall !!!

@sambuddhac sambuddhac merged commit 6e77c7d into GenXProject:develop Feb 26, 2024
5 checks passed
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.

8 participants