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 line implementation #143

Open
wants to merge 23 commits into
base: develop
Choose a base branch
from
Open

Conversation

abhyshr
Copy link
Collaborator

@abhyshr abhyshr commented May 8, 2024

Merge request type

  • [ x] New feature
  • Resolves bug
  • Documentation
  • Other

Relates to

  • [x ] OPFLOW
  • SOPFLOW
  • SCOPFLOW
  • TCOPFLOW
  • [x ] CMake build system
  • Spack configuration
  • Manual
  • Web docs
  • [x ] Tests
  • Other

This MR updates

  • [ x] Header files
  • [ x] Source code
  • CMake build system
  • Spack configuration
  • Web docs
  • Manual
  • Other

Summary
DC line implementation for PFLOW and OPFLOW

Summarize the MR concisely
This PR adds DC line model in PFLOW and OPFLOW. An input file for testing datafiles/case9/case9_dcline.m has been also added along with a toml test and a smoke test.

@abhyshr abhyshr added the opflow Concerning the OPFLOW application label May 8, 2024
Copy link
Contributor

@cameronrutherford cameronrutherford left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably a lot of comments that have simple answers, but this is looking great! Glad the TOML tests are still standing the test of time.

Some additional documentation about this feature would be nice to see in this PR as well

include/constants.h Outdated Show resolved Hide resolved
include/constants.h Outdated Show resolved Hide resolved
include/private/psimpl.h Show resolved Hide resolved
include/private/psimpl.h Show resolved Hide resolved
Comment on lines +410 to +426
PetscInt Nbus, Ngen, Nline, Nload,
Ndcline; /* global # of buses,gens,branches,loads, and dclines (includes
elements which are out of service */
PetscInt nbus, ngen, nline, nload,
ndcline; /* local # of buses,gens,branches,and loads */
PetscInt NlineON, nlineON; /* Number of active lines (includes DC lines) */
PetscInt NdclineON, ndclineON; /* Number of active dc lines */
PetscInt NgenON, ngenON; /* Number of active generators */
PetscInt Nref, nref; /* Number of reference buses */
/* Number of generator types */
PetscInt ngencoal, ngenwind, ngensolar, ngenng, ngennuclear, ngenhydro,
ngenundefined;
/* Number of renewable generators (solar, wind) */
PetscInt ngenrenew;
/* Number of isolated buses */
PetscInt nisolated_buses, Nisolated_buses;
PetscInt *isolated_buses; /* Array to hold isolated buses */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not thrilled about a lot of these variable names. Is there a better way to differentiate / describe these variables? The difference of 1 letter in a different case is easy to typo / miss, and the intention isn't clear from reading.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

N and n are typically used in the context of a parallel implementation where N is the global size and n is the local size. Perhaps, it is confusing here and this should be replaced. But, this is a big change that we need to do through another PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we create an issue for this then? Perhaps naming n_local_... and n_global_... would make more sense? A little more of a cumbersome name, but should be more readable.

src/pflow/pflow.cpp Outdated Show resolved Hide resolved
src/ps/psreaddata.cpp Show resolved Hide resolved
src/ps/psreaddata.cpp Show resolved Hide resolved
src/ps/psreaddata.cpp Outdated Show resolved Hide resolved
src/ps/psreaddata.cpp Show resolved Hide resolved
@abhyshr
Copy link
Collaborator Author

abhyshr commented May 9, 2024

@cameronrutherford : I've addressed your comments. Please take a look and let me know if anything else is needed. Thanks.

@cameronrutherford
Copy link
Contributor

[Error] Problem is infeasible due to inconsistent bounds for the variables (lower>upper). Please fix this. In the meanwhile, HiOp will exit (ungracefully).
[Error] Problem is infeasible due to inconsistent bounds for the variables (lower>upper). Please fix this. In the meanwhile, HiOp will exit (ungracefully).

Lot's of CI failures on both deception and Newell. Hard for me to say what the fix is, but can't really merge this until tests are passing. Would love to see some documentation as well.

@abhyshr
Copy link
Collaborator Author

abhyshr commented May 24, 2024

@cameronrutherford : I have reverted the angle limits back to negative and positive infinity. Hopefully, this should fix the tests.

@cameronrutherford
Copy link
Contributor

@abhyshr still lots of errors in CI tests EXIT: Problem has inconsistent variable bounds or constraint sides.

We can easily update the tests to respect the new limit if you want to revert back to your original change, but these Ipopt based errors seem to be independent from your change

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
opflow Concerning the OPFLOW application
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants