Skip to content

Minimal corrections to Phasor Dynamics models for validation - #538

Open
lukelowry wants to merge 3 commits into
developfrom
lukel/bug-fix-dev
Open

Minimal corrections to Phasor Dynamics models for validation#538
lukelowry wants to merge 3 commits into
developfrom
lukel/bug-fix-dev

Conversation

@lukelowry

Copy link
Copy Markdown
Collaborator

Description

Validated against PowerWorld. Less than 0.5% relative error on WECC 240 case, ACTIVSg200 case, and ACTIVSg500 case.

Further comments

Other open-source grid modeling frameworks do not even come close to this low a validation error against their validation tools, especially not for cases this large. I don't want perfect to be the enemy of good + we can do cleanup more confidently after merging #461.

@nkoukpaizan

Copy link
Copy Markdown
Collaborator

Is this meant to supersede #525 ?

@lukelowry

Copy link
Copy Markdown
Collaborator Author

Is this meant to supersede #525 ?

Yes, I forgot to mention that

@nkoukpaizan nkoukpaizan changed the title Minimal corrections Minimal corrections to Phasor Dynamics models for validation Aug 13, 2026
@nkoukpaizan nkoukpaizan added this to the Release 0.2 milestone Aug 13, 2026

@nkoukpaizan nkoukpaizan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

A few comments. Otherwise looks good!

Comment on lines +315 to +319
RealT ke0 = Ke_;
if (ke0 == ZERO<RealT>)
{
ke0 = (Vrmax_ / 10.0 - static_cast<RealT>(se0)) / static_cast<RealT>(efdp0);
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think this still qualifies as changing the input parameter (Ke_), and this is not in the README. Can we avoid it in the PR and successfully validate?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This is one of the few that I was unsure about in our meeting today. I think that if we instead define a derived parameter as I did in 9082c8a, this will hopefully resolve any confusion. It's not so much changing the parameter as deriving a new parameter. Let me know what you think of my changes

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Understood. In that case, adding this to the Model Derived Parameters section of the documentation should help.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Please see changes in dbbafb2

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

In #525 parameter Ke_ was overwritten, now the new value is stored in Ke_eff_ and is used instead of Ke_. Unless the new parameter Ke_eff_ has well defined physical meaning and the governing physics is documented in the README file, this change will fall outside the scope of this PR.

Comment thread GridKit/Model/PhasorDynamics/Exciter/ESDC1A/README.md Outdated
Comment thread GridKit/Model/PhasorDynamics/Governor/Tgov1/README.md Outdated
Comment thread GridKit/Model/PhasorDynamics/Exciter/IEEET1/Ieeet1Impl.hpp

@abirchfield abirchfield left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

A good compromise that addresses the issues raised in #525 while allowing us to have excellent validation in the short term. Exciting that we are almost ready to complete the 10k case.

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.

4 participants