Skip to content

Gen classical #94

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

Closed
wants to merge 100 commits into from
Closed

Gen classical #94

wants to merge 100 commits into from

Conversation

abdourahmanbarry
Copy link
Collaborator

@abdourahmanbarry abdourahmanbarry commented May 3, 2025

Description

Implemented the second-order classical generator model as a Phasor Dynamics family of components. This model has been used to simulate a 2-bus example and the results outputted to a .csv file. New CMake files have been added as needed, and existing ones have been updated to incorporate the new additions.

This closes #73

Proposed changes

I have added a test case that sets up the system with a consistent DAE solution through the initialize method of the classical generator model and checks that the residual is zero. Another test case has been added to verify that the residuals are correct for other states different from the initialization. Additionally, the CMake files have been updated, and I have built and verified that all configurations work correctly.

Checklist

  • Generator model implementation
  • Proper CMake build configuration.
  • Documentation in a form of README file with model equations.
  • Example of usage.
  • All tests pass.
  • Code compiles cleanly with flags -Wall -Wpedantic -Wconversion -Wextra.
  • The new code follows GridKit™ style guidelines.
  • There are unit tests for the new code.
  • The new code is documented.
  • The feature branch is rebased with respect to the target branch.

Copy link
Collaborator

@abirchfield abirchfield left a comment

Choose a reason for hiding this comment

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

I would move the example to PhasorDynamics. Other than that it looks good.

@lukelowry
Copy link
Collaborator

Small suggestion, but for admittance values use capital letters $g\mapsto G$ and $b\mapsto B$

No other suggestions other than documentation, good job sir.

@pelesh
Copy link
Collaborator

pelesh commented May 7, 2025

Quick comment: @abirchfield and @lukelowry suggested GenClassical as the name for this model. See generator documentation.

Can we agree on that name and use it consistently across the GridKit code?

@abdourahmanbarry
Copy link
Collaborator Author

Thank you all for the feedback. I have incorporated all the suggestions. Please let me know if any further changes are needed.

Copy link
Collaborator

@pelesh pelesh left a comment

Choose a reason for hiding this comment

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

Very nice work! A few minor things to address, mainly about designing tests.

Also, there are merge conflicts that need to be resolved. The feature branch needs to be rebased with respect to develop.

Comment on lines 38 to 43
Bus<double, size_t> bus1(0.9949877346411762, 0.09999703952427966);
BusInfinite<double, size_t> bus2(1.0, 0.0);
Branch<double, size_t> branch(&bus1, &bus2, 0.0, 0.1, 0, 0);
Copy link
Collaborator

Choose a reason for hiding this comment

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

A comment would be helpful to explain why Bus-1 is initialized like this. The numbers are not intuitive.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am not sure I have good explanation for why the numbers are the way they are. I copied that from one of Adam's examples. Maybe if you have good numbers, I can use those instead.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The other example was for a different machine. I am not sure it applies here. I would try first Vr = 1 and Vi =0 and see if the solver can converge to the steady-state solution from there.

CC @abirchfield

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I used Vr = 1 and Vi = 0, and it looks like the solution is approaching the steady-state solution. See the graph below:

steadystate

@pelesh
Copy link
Collaborator

pelesh commented May 22, 2025

Rebased to the most recent develop.

@shakedregev
Copy link
Collaborator

      Start 13: AdjointSens
13/29 Test #13: AdjointSens ......................***Exception: SegFault  0.23 sec

@nkoukpaizan
Copy link
Collaborator

  • CPU tests (Ubunutu x86_64) / Build gridkit with Spack (16, gridkit@develop +enzyme ^[email protected] ^sundials@develop) (pull_request)

@shakedregev You might want sundials@develop (or a the specific commit) for this. See #118.

@pelesh pelesh mentioned this pull request Jun 4, 2025
6 tasks
@pelesh
Copy link
Collaborator

pelesh commented Jun 4, 2025

Closing in favor of #127

@pelesh pelesh closed this Jun 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement classical generator model in phasor-domain dynamics familly
6 participants