Global Views in PhasorDynamics SystemModel using new Vector by lukelowry · Pull Request #452 · ORNL/GridKit

lukelowry · 2026-06-16T15:16:02Z

Description

This PR updates PhasorDynamics so buses and components use their slices of the SystemModel state vectors directly. This removes the local state copy path and keeps component accessors consistent with the integrator-updated system state.

Partially addresses #96. Related to #145.

Proposed changes

Added StateVector support so GridElement can own standalone state or alias SystemModel state.
Updated SystemModel allocation to bind each bus/component to global state and verify element sizes remain fixed.
Updated Enzyme call sites to use the explicit yData() accessor for live state pointers.
Added a PhasorDynamics system test for element/system state aliasing.

Performance was measured with application/PhasorDynamics/PDSim using the app-reported Complete in time. Each row reports the median of 3 trials after rebuilding PDSim on the corresponding branch.

Case	`develop` (s)	This branch (s)	Improvement
Hawaii	0.201302	0.165187	17.9%
NewEngland	0.272596	0.220406	19.1%
Illinois	0.664262	0.592922	10.7%
WECC	1.203380	1.045540	13.1%
Texas	9.063860	7.515690	17.1%

Checklist

All tests pass. (except the known PE failure)
Code compiles cleanly with flags -Wall -Wpedantic -Wconversion -Wextra.
The new code follows GridKit(TM) 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.
N/A: no CHANGELOG.md update is needed for this internal PhasorDynamics state-storage change.

Further comments

This work was motivated by the addition of IEEEST stabilizers: adding 333 stabilizers to the Texas case dramatically reduced performance, and this change removes avoidable state-copy overhead in PhasorDynamics system evaluation.

NOTE: I had to make a new PR but this is revival of #411

PhilipFackler · 2026-06-18T18:07:17Z

I like the VectorView class. I don't think this is the right way to use it. We could use it in the way that, while touching a lot of files outside PhasorDynamics, would only change the API (not the implementation) for those classes. I'm going to explain my thinking even though, given your comments in the last meeting, you've probably had these ideas already. :)

The abstract functions defined for Evaluator should return one of these.

That is,

      virtual std::vector<ScalarT>&       y()       = 0;
      virtual const std::vector<ScalarT>& y() const = 0;

becomes

      virtual VectorView<ScalarT>       y()       = 0;  // no `&` needed
      virtual VectorView<const ScalarT> y() const = 0;

This means that the Evaluator API doesn't care if a subclass is owning or non-owning.

VectorView would need (non-explicit) constructor(s) to convert from [const] std::vector<T>&. This would allow for Evaluators outside PhasorDynamics to still return their std::vector members from those Evaluator functions.
In PhasorDynamics, Component (and BusBase) can use Slaven's Vector directly for these variables. SystemModel as a Component would own its data in the normal use-case, and components would not. (This also makes conceivable the possibility of a SystemModel also being non-owning in a hierarchical system.)
Then we would just need a conversion from Vector to VectorView; either another constructor for VectorView or a method on Vector (Vector::getView())...or both.

pelesh · 2026-06-24T22:11:25Z

I suggest removing View class altogether and using Vector instead. If additional features are required in Vector class, please create an issue.

lukelowry · 2026-06-25T02:14:19Z

@pelesh @PhilipFackler thank you both for your suggestions. I tried my hardest to take the best of both solutions to get the best of both. Please let me know what you think.

lukelowry · 2026-06-26T20:00:47Z

Rebased, but the speedup is not as significant because the scatter of the jacobian is is now much larger from recent changes in develop :(

pelesh

For some reason I cannot build this branch. When building from scratch, I get following compile error:

Details

``` [ 5%] Built target phasor_dynamics_signal_dependency_tracking Undefined symbols for architecture arm64: "GridKit::LinearAlgebra::Vector::getData(GridKit::LinearAlgebra::memory::MemorySpace)", referenced from: AnalysisManager::Sundials::Kinsol::Residual(_generic_N_Vector*, _generic_N_Vector*, void*) in Kinsol.cpp.o AnalysisManager::Sundials::Kinsol::copyVec(_generic_N_Vector*, GridKit::LinearAlgebra::Vector&) in Kinsol.cpp.o "GridKit::LinearAlgebra::Vector::getData(GridKit::LinearAlgebra::memory::MemorySpace)", referenced from: AnalysisManager::Sundials::Kinsol::Residual(_generic_N_Vector*, _generic_N_Vector*, void*) in Kinsol.cpp.o AnalysisManager::Sundials::Kinsol::copyVec(_generic_N_Vector*, GridKit::LinearAlgebra::Vector&) in Kinsol.cpp.o "GridKit::LinearAlgebra::Vector::getData(GridKit::LinearAlgebra::memory::MemorySpace)", referenced from: AnalysisManager::Sundials::Kinsol::Residual(_generic_N_Vector*, _generic_N_Vector*, void*) in Kinsol.cpp.o AnalysisManager::Sundials::Kinsol::copyVec(_generic_N_Vector*, GridKit::LinearAlgebra::Vector&) in Kinsol.cpp.o "GridKit::LinearAlgebra::Vector::getData(GridKit::LinearAlgebra::memory::MemorySpace) const", referenced from: AnalysisManager::Sundials::Kinsol::Residual(_generic_N_Vector*, _generic_N_Vector*, void*) in Kinsol.cpp.o AnalysisManager::Sundials::Kinsol::getDefaultInitialCondition() in Kinsol.cpp.o AnalysisManager::Sundials::Kinsol::copyVec(GridKit::LinearAlgebra::Vector const&, _generic_N_Vector*) in Kinsol.cpp.o "GridKit::LinearAlgebra::Vector::getSize() const", referenced from: AnalysisManager::Sundials::Kinsol::Residual(_generic_N_Vector*, _generic_N_Vector*, void*) in Kinsol.cpp.o AnalysisManager::Sundials::Kinsol::Residual(_generic_N_Vector*, _generic_N_Vector*, void*) in Kinsol.cpp.o AnalysisManager::Sundials::Kinsol::getDefaultInitialCondition() in Kinsol.cpp.o AnalysisManager::Sundials::Kinsol::copyVec(GridKit::LinearAlgebra::Vector const&, _generic_N_Vector*) in Kinsol.cpp.o AnalysisManager::Sundials::Kinsol::copyVec(_generic_N_Vector*, GridKit::LinearAlgebra::Vector&) in Kinsol.cpp.o "GridKit::LinearAlgebra::Vector::getData(GridKit::LinearAlgebra::memory::MemorySpace) const", referenced from: AnalysisManager::Sundials::Kinsol::Residual(_generic_N_Vector*, _generic_N_Vector*, void*) in Kinsol.cpp.o AnalysisManager::Sundials::Kinsol::getDefaultInitialCondition() in Kinsol.cpp.o AnalysisManager::Sundials::Kinsol::copyVec(GridKit::LinearAlgebra::Vector const&, _generic_N_Vector*) in Kinsol.cpp.o "GridKit::LinearAlgebra::Vector::getSize() const", referenced from: AnalysisManager::Sundials::Kinsol::Residual(_generic_N_Vector*, _generic_N_Vector*, void*) in Kinsol.cpp.o AnalysisManager::Sundials::Kinsol::Residual(_generic_N_Vector*, _generic_N_Vector*, void*) in Kinsol.cpp.o AnalysisManager::Sundials::Kinsol::getDefaultInitialCondition() in Kinsol.cpp.o AnalysisManager::Sundials::Kinsol::copyVec(GridKit::LinearAlgebra::Vector const&, _generic_N_Vector*) in Kinsol.cpp.o AnalysisManager::Sundials::Kinsol::copyVec(_generic_N_Vector*, GridKit::LinearAlgebra::Vector&) in Kinsol.cpp.o "GridKit::LinearAlgebra::Vector::getData(GridKit::LinearAlgebra::memory::MemorySpace) const", referenced from: AnalysisManager::Sundials::Kinsol::Residual(_generic_N_Vector*, _generic_N_Vector*, void*) in Kinsol.cpp.o AnalysisManager::Sundials::Kinsol::getDefaultInitialCondition() in Kinsol.cpp.o AnalysisManager::Sundials::Kinsol::copyVec(GridKit::LinearAlgebra::Vector const&, _generic_N_Vector*) in Kinsol.cpp.o "GridKit::LinearAlgebra::Vector::getSize() const", referenced from: AnalysisManager::Sundials::Kinsol::Residual(_generic_N_Vector*, _generic_N_Vector*, void*) in Kinsol.cpp.o AnalysisManager::Sundials::Kinsol::Residual(_generic_N_Vector*, _generic_N_Vector*, void*) in Kinsol.cpp.o AnalysisManager::Sundials::Kinsol::getDefaultInitialCondition() in Kinsol.cpp.o AnalysisManager::Sundials::Kinsol::copyVec(GridKit::LinearAlgebra::Vector const&, _generic_N_Vector*) in Kinsol.cpp.o AnalysisManager::Sundials::Kinsol::copyVec(_generic_N_Vector*, GridKit::LinearAlgebra::Vector&) in Kinsol.cpp.o ld: symbol(s) not found for architecture arm64 [ 5%] Built target utilities_logger clang++: error: linker command failed with exit code 1 (use -v to see invocation) make[2]: *** [GridKit/Solver/SteadyState/libgridkit_solvers_steady.0.1.0.dylib] Error 1 make[1]: *** [GridKit/Solver/SteadyState/CMakeFiles/solvers_steady.dir/all] Error 2 make[1]: *** Waiting for unfinished jobs.... ```

lukelowry · 2026-07-06T14:30:45Z

@pelesh i think it was a linker issue, fixed in recent commit. Could you try again?

pelesh · 2026-07-06T14:39:49Z

@pelesh i think it was a linker issue, fixed in recent commit. Could you try again?

That fixed it. Thanks!

pelesh

This is great work and I suggest we make it top priority for merging.

I suggest we complete all necessary Vector class and other infrastructure changes in #477 and have this PR address redundant copies. That way we avoid scope creep here.

lukelowry · 2026-07-08T15:49:34Z

I will rebase as soon as I get a chance, high priority

lukelowry · 2026-07-08T22:19:57Z

@pelesh I am marking this ready for review, this is one of the last big things before I can get the new models ready for final review

lukelowry · 2026-07-10T18:11:13Z

This is the last item blocking the completion of the other PR models, (#415 , #421 , etc). Once merged I will open a validation PRs for the example cases and update/standardize all the cases and model implementation for readability!

nkoukpaizan

Mostly looks good to me. A few relatively minor comments.

nkoukpaizan · 2026-07-11T14:29:49Z


  result += test.constructor();
  result += test.composer();
+  result += test.elementStateAliasesSystemState();


Suggested change

result += test.elementStateAliasesSystemState();

result += test.componentStateAliasesSystemState();

nkoukpaizan · 2026-07-11T14:33:12Z

-        auto residual_y = load.getResidual();
+        load.evaluateResidual(); ///< Computes the residual and the Jacobian values by tracking
+                                 ///< the dependencies
+        auto&                                     residual_y_view = load.getResidual();


I'm not sure I understand the _view in the variable name in the context of the current version of the PR.

nkoukpaizan · 2026-07-11T14:38:53Z

                                    ///< the dependencies
-        std::vector<DependencyTracking::Variable> residual_yp = exciter.getResidual();
+        auto&                                     residual_yp_view = exciter.getResidual();
+        std::vector<DependencyTracking::Variable> residual_yp(residual_yp_view.getData(), residual_yp_view.getData() + residual_yp_view.getSize());


Probably out of scope for this PR, but we'll need to remove the std::vector<DependencyTracking::Variable>. I'll give that some thought.

nkoukpaizan · 2026-07-11T14:47:23Z

      // static void copyMat(Model::Evaluator::Mat& J, SlsMat Jida);
-      static void copyVec(const N_Vector x, std::vector<ScalarT>& y);
-      static void copyVec(const std::vector<ScalarT>& x, N_Vector y);
-      static void copyVec(const std::vector<bool>& x, N_Vector y);


If VectorT is GridKit::LinearAlgebra::Vector<ScalarT, IdxT>, what's the behavior for the tag (bool)? An implicit conversion seems dangerous to me, but looks like IDA is expecting a floating point value anyway. cc @Steven-Roberts

nkoukpaizan · 2026-07-11T15:03:50Z

    }

  private:
+    static ScalarT* dataOrNull(VectorT& vector)


This is not as elegant as what is being done in PhasorDynamics to explicitly check the size.

Trying to access the nullptr is undefined behavior. We'd have to check if the return value is nullptr before using.

nkoukpaizan · 2026-07-11T15:23:28Z

+   * @return true if the write was successful, false otherwise
+   */
+  template <typename VectorT>
+  void writeVectorToMatrixMarket(const VectorT& vec, const std::string& filename, const std::string& header)


I thought I had deleted this in #447. Rebasing issue perhaps?

I think this belongs either in the Vector class or a separate utility. Tracking the task of adding it back in #467.

nkoukpaizan · 2026-07-11T15:28:28Z

      int evaluateJacobian() override;

-      void updateVariables();
+      CsrMatrixT* getCsrJacobian() const override


I believe I moved this to the Component base class and it should not need an override here.

nkoukpaizan · 2026-07-11T15:31:29Z

  {
-    y[i]   = dg->y()[i];
-    res[i] = dg->getResidual()[i];
+    y[i]   = dg->y().getData()[i];


The pattern auto* y_data = dg->y().getData(); y_data[i]... (used elsewhere) looks cleaner to me. .getData()[1] looks cleaner if we are only accessing a specific index.

nkoukpaizan · 2026-07-11T15:47:08Z

+      assert(child_offset == size_);

-      // Default variable and residual index mapping to local index
+      variable_indices_.resize(size);


It may be a good idea to make these GridKit::LinearAlgebra::Vector<IdxT, IdxT> right away and allocate them with the others.

nkoukpaizan · 2026-07-11T15:50:48Z

      }

-      [[noreturn]] std::vector<ScalarT>& yB() override
+      [[noreturn]] VectorT& yB() override


The member variables should also be updated, though they are not used. Please add a TODO around those member variables, so we know to pay attention to them later.

lukelowry force-pushed the lukel/perf-sys-dev branch 3 times, most recently from e7e6b07 to 5d4f6ff Compare June 22, 2026 16:34

pelesh self-requested a review June 24, 2026 22:07

lukelowry force-pushed the lukel/perf-sys-dev branch 3 times, most recently from 2955062 to b70b473 Compare June 25, 2026 02:05

lukelowry force-pushed the lukel/perf-sys-dev branch 2 times, most recently from 0231d24 to de9a228 Compare June 26, 2026 19:16

lukelowry force-pushed the lukel/perf-sys-dev branch 2 times, most recently from 3efa5b9 to 57e09d3 Compare July 1, 2026 05:17

pelesh reviewed Jul 3, 2026

View reviewed changes

pelesh mentioned this pull request Jul 4, 2026

Vector and supporting classes update #477

Merged

7 tasks

pelesh reviewed Jul 6, 2026

View reviewed changes

Comment thread GridKit/LinearAlgebra/Vector/Vector.hpp Outdated

Comment thread GridKit/LinearAlgebra/Vector/Vector.hpp Outdated

lukelowry force-pushed the lukel/perf-sys-dev branch from 3e44075 to d2636cc Compare July 8, 2026 19:36

lukelowry marked this pull request as ready for review July 8, 2026 22:19

lukelowry force-pushed the lukel/perf-sys-dev branch from e668c7d to 1827b6a Compare July 9, 2026 01:23

lukelowry and others added 5 commits July 9, 2026 11:42

Migrate Evaluator state to View; auto-bind in allocate

2d066cf

Apply pre-commit fixes

21c76a6

Adjusted ida test

7bce770

linker fixes

1749955

fix namespace references from rebase

e1b3ebb

lukelowry added 6 commits July 9, 2026 11:42

remove uncessary changes to Vector class

876a755

remoe data() use getData instead

e7ecd9e

remove operator and use getData

0603c3e

remove size alias

1839fd2

reduce churnon Vector

03957e2

ensure non-zero sized component bug

93a6fc5

lukelowry force-pushed the lukel/perf-sys-dev branch from 1827b6a to 93a6fc5 Compare July 9, 2026 17:11

nkoukpaizan assigned lukelowry Jul 10, 2026

nkoukpaizan requested review from PhilipFackler and nkoukpaizan July 10, 2026 14:08

nkoukpaizan added the enhancement New feature or request label Jul 10, 2026

nkoukpaizan reviewed Jul 11, 2026

View reviewed changes

	result += test.elementStateAliasesSystemState();
	result += test.componentStateAliasesSystemState();

Uh oh!

Conversation

lukelowry commented Jun 16, 2026

Description

Proposed changes

Checklist

Further comments

Uh oh!

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

Uh oh!

Uh oh!

pelesh commented Jun 24, 2026

Uh oh!

lukelowry commented Jun 25, 2026

Uh oh!

lukelowry commented Jun 26, 2026

Uh oh!

pelesh left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lukelowry commented Jul 6, 2026

Uh oh!

pelesh commented Jul 6, 2026

Uh oh!

pelesh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

lukelowry commented Jul 8, 2026

Uh oh!

lukelowry commented Jul 8, 2026

Uh oh!

lukelowry commented Jul 10, 2026

Uh oh!

nkoukpaizan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

PhilipFackler commented Jun 18, 2026 •

edited

Loading

pelesh left a comment •

edited

Loading