Skip to content

Lukel/phasor docs #88

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

Merged
merged 48 commits into from
May 2, 2025
Merged

Lukel/phasor docs #88

merged 48 commits into from
May 2, 2025

Conversation

lukelowry
Copy link
Collaborator

@lukelowry lukelowry commented Apr 22, 2025

Description

The documentation for the GENROU and GENSAL (Synchronous Machine models) has been reviewed and modified to be consistent with the implementation.

This closes issue #79

Proposed changes

The README equations and nomenclature notation is derived directly from the C++ implementation. Information from the general Synchronous Machine README has been removed and put into the relevant sub-models. Since we will be implementing other classes of synch. generator models, so the GENROU and GENSAL equations should not be inn the folders general README.

Further comments

Let me know if there was any notation violations, the guidelines don't seem to have much in regards to math notation.

@lukelowry lukelowry linked an issue Apr 22, 2025 that may be closed by this pull request
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.

Overall looks good. Need to make most of the changes Slaven suggested and one other about the saturation.

@lukelowry
Copy link
Collaborator Author

Branch is up to date with requested edits. Thank you for the feedback!

We can extend explanations further, just please identify what could benefit from enrichment.

@abirchfield
Copy link
Collaborator

Move 4 differential variables back to differential as discussed

@pelesh
Copy link
Collaborator

pelesh commented May 2, 2025

Move 4 differential variables back to differential as discussed

Correction is needed for both, GENROU and GENSAL docs.

@lukelowry
Copy link
Collaborator Author

Okay, ready for approval. I will create new branch for exciter & governor models moving forward and use these models as reference for formatting.

Copy link
Collaborator

@abdourahmanbarry abdourahmanbarry left a comment

Choose a reason for hiding this comment

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

Looks very good, Luke. I have identified three typos to be fixed.

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.

Please check highlighted sign issues.

@lukelowry
Copy link
Collaborator Author

Changes applied, thank you.

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.

Good to go ;)

@pelesh pelesh merged commit 77c973e into develop May 2, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Document PhasorDynamics::Genrou model
4 participants