IEEEST Stabilizer Improvements/Corrections by lukelowry · Pull Request #460 · ORNL/GridKit

lukelowry · 2026-06-22T16:49:18Z

Description

Fixes and cleans up the IEEEST stabilizer implementation and documentation.

This updates the older initial implementation style so that reduced-order notch filters and zero denominator time constants are handled consistently

Proposed changes

Replace the branchy/safe-inverse notch logic with a fixed DAE form using differential tags.
Initialize IEEEST from the attached input signal instead of forcing all states to zero.
Document the implemented equations, initialization, derived parameters, and unsupported compatibility parameters.
Tests for initialization, residuals, invalid wiring, invalid notch parameters, zero time constants, and Jacobian consistency.

Checklist

All tests pass for the focused PhasorDynamics coverage.
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.

Changelog changes N/A

Further comments

This is cleanup/correction work for the existing IEEEST model, not a new model

lukelowry · 2026-06-22T18:44:31Z

@pelesh this fix is also ready to be merged I forgot to make a PR for this one.

pelesh

The logic governing changing differential into algebraic equations and vice versa does not seem to be sustainable. This requires more offline discussion.

lukelowry · 2026-06-26T12:48:23Z

I have rebased and adjusted the formulation to align with #463 .

nkoukpaizan

I have a few comments here. Perhaps worth discussing at our next meeting.

nkoukpaizan · 2026-06-28T22:39:34Z

        using Variable = typename ModelDataT::MonitorableVariables;
-        monitor_->set(Variable::vss, [this]
-                      { return y_[11]; });
+        auto index     = [](IeeestInternalVariables variable)


I've seen this pattern a few times. Consider defining once if it is really needed. I would argue it's less readable.

lukelowry · 2026-07-02T15:43:03Z

Testing Ground for template parameters <Order N>

lukelowry · 2026-07-08T22:25:20Z

general implementation of the template parameter done

lukelowry · 2026-07-20T15:13:53Z

@nkoukpaizan what are your thoughts on my template implementation?

nkoukpaizan · 2026-07-21T17:43:21Z

@nkoukpaizan what are your thoughts on my template implementation?

The model implementation is cleaner with the order as a template parameter. However, the order remains a runtime selection based on floating point comparisons, which is what I wanted to avoid.

lukelowry · 2026-07-21T18:40:44Z

@nkoukpaizan we could possibly require the order to be passed as a parameter of IEEEST model, would that resolve your concern?

"Order" is not usually directly exposed as a model parameter and so would require some knowledge of how IEEEST works to pass valid parameters.

Can you give an example where the runtime comparison is bad/fails? I might be able to resolve both our concerns if I understand better.

nkoukpaizan · 2026-07-21T19:03:05Z

@nkoukpaizan we could possibly require the order to be passed as a parameter of IEEEST model, would that resolve your concern?

"Order" is not usually directly exposed as a model parameter and so would require some knowledge of how IEEEST works to pass valid parameters.

Can you give an example where the runtime comparison is bad/fails? I might be able to resolve both our concerns if I understand better.

Yes, exposing the order as a parameter to the model is what I had in mind. Understood that that's not the domain practice. That's why as was asking whether this is possible as a question.

Floating point values are stored with limited precision. We'd have to guarantee that the value returned by the parser is bit-for-bit equal to zero. That may be the case for nlohmann-json, but considering #227 and the possibility of using a different parser, that's not guaranteed.

lukelowry · 2026-07-22T05:58:40Z

@nkoukpaizan i see the issue now thank you, that was very clarifying. After reflecting on my past experience and discussing with others at TAMU, I think deriving the order from the parameters alone is not intuitive or safe, either.

As discussed, we should add an integer parameter to the IEEEST model, "order" and use that for the StabilizerFactory (rename to IeeestFactory). Then, our parameter validation can gate for zero division relevant to each static order (e.g. $A_2 + A_3>0$). Additionally the model json may then contain some coefficients that are only used at a certain "order", which would be a feature IMO since you can retain information needed for higher order but choose to operate at smaller order.

I will make this change then we can reasess.

superwhiskers · 2026-09-10T19:15:20Z

+        void initializeMonitor();
+        void setDerivedParameters();
+
+        static constexpr RealT TIME_CONSTANT_MINIMUM = static_cast<RealT>(1.0e-3);


it looks like this same constant is duplicated across many header files. is there any reason it cannot be placed in a more "global" place so we aren't copy-pasting it all over?

Consolidating is desirable and possible. But at this point, it's still not clear that we want to maintain this minimum time constant.

superwhiskers

nothing significant, just some additional comments & suggestions

superwhiskers · 2026-09-24T19:36:38Z

+    template <typename scalar_type, typename index_type>
+    class SignalNode;
+


why are we forward declaring this here?

superwhiskers · 2026-09-24T19:37:19Z


        Ieeest();
-        Ieeest(const ModelDataT& data);
+        explicit Ieeest(const ModelDataT& data);


just curious what motivated this adjustment here

superwhiskers · 2026-09-24T19:45:01Z

+        auto check = [&](bool condition, const char* message)
+        {
+          if (!condition)
+          {
+            Log::error() << "Ieeest: " << message << '\n';
+            ret += 1;
+          }
+        };


may be worthwhile building on top of your #566 pull request. could rebase this upon that branch

lukelowry · 2026-10-11T19:09:11Z

Closing and continuing in fresh PR #617 for sanity and clarity per @pelesh

lukelowry marked this pull request as ready for review June 22, 2026 18:43

lukelowry mentioned this pull request Jun 23, 2026

10k Bus Case + Three Fully Validated Cases #461

Merged

6 tasks done

lukelowry force-pushed the lukel/ieeest-dev branch from 509506c to e2c07d8 Compare June 24, 2026 20:00

pelesh requested changes Jun 24, 2026

View reviewed changes

Comment thread GridKit/Model/PhasorDynamics/Stabilizer/IEEEST/IeeestImpl.hpp Outdated

Comment thread GridKit/Model/PhasorDynamics/Stabilizer/IEEEST/IeeestImpl.hpp Outdated

Comment thread GridKit/Model/PhasorDynamics/Stabilizer/IEEEST/IeeestImpl.hpp Outdated

lukelowry force-pushed the lukel/ieeest-dev branch from e2c07d8 to bab57ad Compare June 24, 2026 22:11

pelesh mentioned this pull request Jun 25, 2026

Make PhasorDynamics models Hessenberg-ready #463

Open

lukelowry force-pushed the lukel/ieeest-dev branch 2 times, most recently from 8bbbad6 to 090289b Compare June 26, 2026 11:02

lukelowry force-pushed the lukel/ieeest-dev branch from 755ee20 to 282fc1d Compare June 26, 2026 21:59

nkoukpaizan reviewed Jun 28, 2026

View reviewed changes

lukelowry force-pushed the lukel/ieeest-dev branch from 282fc1d to 579b3c1 Compare July 1, 2026 05:17

lukelowry added this to the WECC Completion milestone Jul 2, 2026

lukelowry force-pushed the lukel/ieeest-dev branch from 579b3c1 to 9fb15fa Compare July 3, 2026 00:25

lukelowry force-pushed the lukel/ieeest-dev branch from 7336d1f to 1104b91 Compare July 9, 2026 16:09

nkoukpaizan added enhancement New feature or request cleanup labels Jul 10, 2026

lukelowry force-pushed the lukel/ieeest-dev branch 4 times, most recently from d0dc95a to d2be200 Compare July 17, 2026 21:58

lukelowry force-pushed the lukel/ieeest-dev branch from d2be200 to 47a31f1 Compare July 22, 2026 06:46

lukelowry marked this pull request as draft September 1, 2026 16:03

lukelowry force-pushed the lukel/ieeest-dev branch 4 times, most recently from bf71cd5 to 07d0236 Compare September 6, 2026 07:08

lukelowry force-pushed the lukel/ieeest-dev branch from 07d0236 to dc2215f Compare September 9, 2026 20:19

superwhiskers reviewed Sep 10, 2026

View reviewed changes

lukelowry force-pushed the lukel/ieeest-dev branch 2 times, most recently from 1d70ce7 to 87c2bd1 Compare September 13, 2026 01:36

pelesh removed this from the WECC Completion milestone Sep 17, 2026

lukelowry force-pushed the lukel/ieeest-dev branch 3 times, most recently from a7c98fd to 258187d Compare September 22, 2026 03:02

superwhiskers reviewed Sep 24, 2026

View reviewed changes

lukelowry force-pushed the lukel/ieeest-dev branch 3 times, most recently from 1a1c488 to 0c6efb2 Compare October 7, 2026 20:26

lukelowry and others added 9 commits October 9, 2026 17:35

IEEEST Stabilizer Improvements/Corrections

d8266f3

remove param names from signature

b21f01c

no ternary operators

e5ce7d1

remove operatorname maybe fix gh render

f7a9f36

test verbosity

3cb8e8d

remove single use method

a9695a1

Apply pre-commit fixes

ba27672

Apply pre-commit fixes

137647f

Apply pre-commit fixes

06cb926

lukelowry force-pushed the lukel/ieeest-dev branch from 0c6efb2 to 06cb926 Compare October 10, 2026 21:02

lukelowry mentioned this pull request Oct 11, 2026

Template IEEEST on its derived notch order #617

Closed

6 tasks done

lukelowry closed this Oct 11, 2026

		template <typename scalar_type, typename index_type>
		class SignalNode;

Conversation

lukelowry commented Jun 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Proposed changes

Checklist

Further comments

Uh oh!

lukelowry commented Jun 22, 2026

Uh oh!

pelesh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lukelowry commented Jun 26, 2026

Uh oh!

nkoukpaizan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nkoukpaizan Jun 28, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lukelowry commented Jul 2, 2026

Uh oh!

lukelowry commented Jul 8, 2026

Uh oh!

lukelowry commented Jul 20, 2026

Uh oh!

nkoukpaizan commented Jul 21, 2026

Uh oh!

lukelowry commented Jul 21, 2026

Uh oh!

nkoukpaizan commented Jul 21, 2026

Uh oh!

lukelowry commented Jul 22, 2026

Uh oh!

superwhiskers Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

nkoukpaizan Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

superwhiskers left a comment

Choose a reason for hiding this comment

Uh oh!

superwhiskers Sep 24, 2026

Choose a reason for hiding this comment

Uh oh!

superwhiskers Sep 24, 2026

Choose a reason for hiding this comment

Uh oh!

superwhiskers Sep 24, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lukelowry commented Oct 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

lukelowry commented Jun 22, 2026 •

edited

Loading