Skip to content

Remove COO_Matrix from PowerElectronics module#361

Merged
pelesh merged 6 commits intodevelopfrom
alex/remove-coo
Apr 7, 2026
Merged

Remove COO_Matrix from PowerElectronics module#361
pelesh merged 6 commits intodevelopfrom
alex/remove-coo

Conversation

@alexander-novo
Copy link
Copy Markdown
Collaborator

@alexander-novo alexander-novo commented Apr 6, 2026

Description

Closes #351

Proposed changes

COO Jacobian values for components are stored in three new buffers jacobian_coo_rows_, jacobian_coo_cols_, jacobian_coo_values_. To minimize changes to models, a helper function CircuitComponent::setJacValues is added with a similar interface as the old COO_Matrix::setValues() which fills these new buffers. nnz_ is also properly added to all of the PowerElectronics models with Jacobians.

Checklist

  • 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.
  • I have updated CHANGELOG.md to reflect the changes in this PR. If this is a minor PR that is part of a larger fix already included in the file, state so.

Further comments

@alexander-novo
Copy link
Copy Markdown
Collaborator Author

Let me know if this needs to be in the changelog

@alexander-novo
Copy link
Copy Markdown
Collaborator Author

Let me know what to do with the EnzymePowerElectronicsCheck example @pelesh

@alexander-novo alexander-novo requested a review from pelesh April 6, 2026 20:21
@pelesh
Copy link
Copy Markdown
Collaborator

pelesh commented Apr 6, 2026

Let me know if this needs to be in the changelog

Yes, please!

@pelesh
Copy link
Copy Markdown
Collaborator

pelesh commented Apr 6, 2026

Let me know what to do with the EnzymePowerElectronicsCheck example @pelesh

I would just update that example not to use the old matrix type. It is using analytical Jacobian only as a reference to verify Enzyme computed one.

@pelesh pelesh added the enhancement New feature or request label Apr 6, 2026
Copy link
Copy Markdown
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.

Great job! We can merge it as soon as

  • Failing test is fixed
  • Compiler warnings are fixed (see below)

Compiler warnings:

Details
In file included from /Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/TransmissionLine/TransmissionLine.cpp:2:
In file included from /Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/TransmissionLine/TransmissionLine.hpp:5:
/Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/CircuitComponent.hpp:107:55: warning: implicit conversion changes signedness: 'long' to 'size_t' (aka 'unsigned long') [-Wsign-conversion]
  107 |       jacobian_coo_rows_   = std::make_unique<IdxT[]>(nnz_);
      |                              ~~~                      ^~~~
/Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/TransmissionLine/TransmissionLine.cpp:182:18: note: in instantiation of member function 'GridKit::CircuitComponent<double, long>::allocate' requested here
  182 |   template class TransmissionLine<double, long int>;
      |                  ^
In file included from /Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/TransmissionLine/TransmissionLine.cpp:2:
In file included from /Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/TransmissionLine/TransmissionLine.hpp:5:
/Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/CircuitComponent.hpp:108:55: warning: implicit conversion changes signedness: 'long' to 'size_t' (aka 'unsigned long') [-Wsign-conversion]
  108 |       jacobian_coo_cols_   = std::make_unique<IdxT[]>(nnz_);
      |                              ~~~                      ^~~~
/Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/CircuitComponent.hpp:109:56: warning: implicit conversion changes signedness: 'long' to 'size_t' (aka 'unsigned long') [-Wsign-conversion]
  109 |       jacobian_coo_values_ = std::make_unique<RealT[]>(nnz_);
      |                              ~~~                       ^~~~
[ 25%] Building CXX object GridKit/Model/PowerElectronics/MicrogridLine/CMakeFiles/power_elec_microline.dir/MicrogridLine.cpp.o
/Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/CircuitComponent.hpp:107:55: warning: implicit conversion changes signedness: 'long' to 'size_t' (aka 'unsigned long') [-Wsign-conversion]
  107 |       jacobian_coo_rows_   = std::make_unique<IdxT[]>(nnz_);
      |                              ~~~                      ^~~~
/Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/TransmissionLine/TransmissionLine.cpp:184:18: note: in instantiation of member function 'GridKit::CircuitComponent<GridKit::DependencyTracking::Variable, long>::allocate' requested here
  184 |   template class TransmissionLine<DependencyTracking::Variable, long int>;
      |                  ^
In file included from /Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/TransmissionLine/TransmissionLine.cpp:2:
In file included from /Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/TransmissionLine/TransmissionLine.hpp:5:
/Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/CircuitComponent.hpp:108:55: warning: implicit conversion changes signedness: 'long' to 'size_t' (aka 'unsigned long') [-Wsign-conversion]
  108 |       jacobian_coo_cols_   = std::make_unique<IdxT[]>(nnz_);
      |                              ~~~                      ^~~~
/Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/CircuitComponent.hpp:109:56: warning: implicit conversion changes signedness: 'long' to 'size_t' (aka 'unsigned long') [-Wsign-conversion]
  109 |       jacobian_coo_values_ = std::make_unique<RealT[]>(nnz_);
      |                              ~~~                       ^~~~
[ 26%] Building CXX object GridKit/Model/PowerElectronics/MicrogridBusDQ/CMakeFiles/power_elec_microbusdq.dir/MicrogridBusDQ.cpp.o
[ 26%] Built target power_elec_lineartrasnformer
[ 26%] Built target power_elec_capacitor
[ 26%] Built target power_elec_inductor
[ 26%] Built target power_elec_resistor
[ 26%] Built target power_elec_voltagesource
[ 26%] Built target power_elec_inductionmotor
[ 26%] Built target power_elec_synmachine
[ 27%] Building CXX object GridKit/Solver/SteadyState/CMakeFiles/solvers_steady.dir/Kinsol.cpp.o
[ 27%] Building CXX object GridKit/Solver/Dynamic/CMakeFiles/solvers_dyn.dir/Ida.cpp.o
[ 28%] Building CXX object GridKit/Testing/CMakeFiles/testing.dir/Testing.cpp.o
[ 28%] Built target spmattest
[ 29%] Built target densemattest
[ 30%] Building CXX object examples/PowerFlow/MatPowerTesting/CMakeFiles/test_parse_gen_row.dir/test_parse_gen_row.cpp.o
In file included from /Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/MicrogridLoad/MicrogridLoad.cpp:2:
In file included from /Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/MicrogridLoad/MicrogridLoad.hpp:5:
/Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/CircuitComponent.hpp:107:55: warning: implicit conversion changes signedness: 'long' to 'size_t' (aka 'unsigned long') [-Wsign-conversion]
  107 |       jacobian_coo_rows_   = std::make_unique<IdxT[]>(nnz_);
      |                              ~~~                      ^~~~
/Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/MicrogridLoad/MicrogridLoad.cpp:140:18: note: in instantiation of member function 'GridKit::CircuitComponent<double, long>::allocate' requested here
  140 |   template class MicrogridLoad<double, long int>;
      |                  ^
In file included from /Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/MicrogridLoad/MicrogridLoad.cpp:2:
In file included from /Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/MicrogridLoad/MicrogridLoad.hpp:5:
/Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/CircuitComponent.hpp:108:55: warning: implicit conversion changes signedness: 'long' to 'size_t' (aka 'unsigned long') [-Wsign-conversion]
  108 |       jacobian_coo_cols_   = std::make_unique<IdxT[]>(nnz_);
      |                              ~~~                      ^~~~
/Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/CircuitComponent.hpp:109:56: warning: implicit conversion changes signedness: 'long' to 'size_t' (aka 'unsigned long') [-Wsign-conversion]
  109 |       jacobian_coo_values_ = std::make_unique<RealT[]>(nnz_);
      |                              ~~~                       ^~~~
/Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/CircuitComponent.hpp:107:55: warning: implicit conversion changes signedness: 'long' to 'size_t' (aka 'unsigned long') [-Wsign-conversion]
  107 |       jacobian_coo_rows_   = std::make_unique<IdxT[]>(nnz_);
      |                              ~~~                      ^~~~
/Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/MicrogridLoad/MicrogridLoad.cpp:142:18: note: in instantiation of member function 'GridKit::CircuitComponent<GridKit::DependencyTracking::Variable, long>::allocate' requested here
  142 |   template class MicrogridLoad<DependencyTracking::Variable, long int>;
      |                  ^
In file included from /Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/MicrogridLoad/MicrogridLoad.cpp:2:
In file included from /Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/MicrogridLoad/MicrogridLoad.hpp:5:
/Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/CircuitComponent.hpp:108:55: warning: implicit conversion changes signedness: 'long' to 'size_t' (aka 'unsigned long') [-Wsign-conversion]
  108 |       jacobian_coo_cols_   = std::make_unique<IdxT[]>(nnz_);
      |                              ~~~                      ^~~~
/Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/CircuitComponent.hpp:109:56: warning: implicit conversion changes signedness: 'long' to 'size_t' (aka 'unsigned long') [-Wsign-conversion]
  109 |       jacobian_coo_values_ = std::make_unique<RealT[]>(nnz_);
      |                              ~~~                       ^~~~
[ 31%] Building CXX object examples/PowerFlow/MatPowerTesting/CMakeFiles/test_parse_bus_row.dir/test_parse_bus_row.cpp.o
[ 32%] Building CXX object examples/PowerFlow/MatPowerTesting/CMakeFiles/test_parse_branch_row.dir/test_parse_branch_row.cpp.o
[ 32%] Building CXX object examples/PowerFlow/MatPowerTesting/CMakeFiles/test_parse_gencost_row.dir/test_parse_gencost_row.cpp.o
6 warnings generated.
In file included from /Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/MicrogridLine/MicrogridLine.cpp:2:
In file included from /Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/MicrogridLine/MicrogridLine.hpp:5:
/Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/CircuitComponent.hpp:107:55: warning: implicit conversion changes signedness: 'long' to 'size_t' (aka 'unsigned long') [-Wsign-conversion]
  107 |       jacobian_coo_rows_   = std::make_unique<IdxT[]>(nnz_);
      |                              ~~~                      ^~~~
/Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/MicrogridLine/MicrogridLine.cpp:145:18: note: in instantiation of member function 'GridKit::CircuitComponent<double, long>::allocate' requested here
  145 |   template class MicrogridLine<double, long int>;
      |                  ^
In file included from /Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/MicrogridLine/MicrogridLine.cpp:2:
In file included from /Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/MicrogridLine/MicrogridLine.hpp:5:
/Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/CircuitComponent.hpp:108:55: warning: implicit conversion changes signedness: 'long' to 'size_t' (aka 'unsigned long') [-Wsign-conversion]
  108 |       jacobian_coo_cols_   = std::make_unique<IdxT[]>(nnz_);
      |                              ~~~                      ^~~~
/Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/CircuitComponent.hpp:109:56: warning: implicit conversion changes signedness: 'long' to 'size_t' (aka 'unsigned long') [-Wsign-conversion]
  109 |       jacobian_coo_values_ = std::make_unique<RealT[]>(nnz_);
      |                              ~~~                       ^~~~
6 warnings generated.
/Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/CircuitComponent.hpp:107:55: warning: implicit conversion changes signedness: 'long' to 'size_t' (aka 'unsigned long') [-Wsign-conversion]
  107 |       jacobian_coo_rows_   = std::make_unique<IdxT[]>(nnz_);
      |                              ~~~                      ^~~~
/Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/MicrogridLine/MicrogridLine.cpp:147:18: note: in instantiation of member function 'GridKit::CircuitComponent<GridKit::DependencyTracking::Variable, long>::allocate' requested here
  147 |   template class MicrogridLine<DependencyTracking::Variable, long int>;
      |                  ^
In file included from /Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/MicrogridLine/MicrogridLine.cpp:2:
In file included from /Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/MicrogridLine/MicrogridLine.hpp:5:
/Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/CircuitComponent.hpp:108:55: warning: implicit conversion changes signedness: 'long' to 'size_t' (aka 'unsigned long') [-Wsign-conversion]
  108 |       jacobian_coo_cols_   = std::make_unique<IdxT[]>(nnz_);
      |                              ~~~                      ^~~~
/Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/CircuitComponent.hpp:109:56: warning: implicit conversion changes signedness: 'long' to 'size_t' (aka 'unsigned long') [-Wsign-conversion]
  109 |       jacobian_coo_values_ = std::make_unique<RealT[]>(nnz_);
      |                              ~~~                       ^~~~
6 warnings generated.
In file included from /Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/MicrogridBusDQ/MicrogridBusDQ.cpp:2:
In file included from /Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/MicrogridBusDQ/MicrogridBusDQ.hpp:5:
/Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/CircuitComponent.hpp:107:55: warning: implicit conversion changes signedness: 'long' to 'size_t' (aka 'unsigned long') [-Wsign-conversion]
  107 |       jacobian_coo_rows_   = std::make_unique<IdxT[]>(nnz_);
      |                              ~~~                      ^~~~
/Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/MicrogridBusDQ/MicrogridBusDQ.cpp:122:18: note: in instantiation of member function 'GridKit::CircuitComponent<double, long>::allocate' requested here
  122 |   template class MicrogridBusDQ<double, long int>;
      |                  ^
In file included from /Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/MicrogridBusDQ/MicrogridBusDQ.cpp:2:
In file included from /Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/MicrogridBusDQ/MicrogridBusDQ.hpp:5:
/Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/CircuitComponent.hpp:108:55: warning: implicit conversion changes signedness: 'long' to 'size_t' (aka 'unsigned long') [-Wsign-conversion]
  108 |       jacobian_coo_cols_   = std::make_unique<IdxT[]>(nnz_);
      |                              ~~~                      ^~~~
/Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/CircuitComponent.hpp:109:56: warning: implicit conversion changes signedness: 'long' to 'size_t' (aka 'unsigned long') [-Wsign-conversion]
  109 |       jacobian_coo_values_ = std::make_unique<RealT[]>(nnz_);
      |                              ~~~                       ^~~~
/Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/CircuitComponent.hpp:107:55: warning: implicit conversion changes signedness: 'long' to 'size_t' (aka 'unsigned long') [-Wsign-conversion]
  107 |       jacobian_coo_rows_   = std::make_unique<IdxT[]>(nnz_);
      |                              ~~~                      ^~~~
/Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/MicrogridBusDQ/MicrogridBusDQ.cpp:124:18: note: in instantiation of member function 'GridKit::CircuitComponent<GridKit::DependencyTracking::Variable, long>::allocate' requested here
  124 |   template class MicrogridBusDQ<DependencyTracking::Variable, long int>;
      |                  ^
In file included from /Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/MicrogridBusDQ/MicrogridBusDQ.cpp:2:
In file included from /Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/MicrogridBusDQ/MicrogridBusDQ.hpp:5:
/Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/CircuitComponent.hpp:108:55: warning: implicit conversion changes signedness: 'long' to 'size_t' (aka 'unsigned long') [-Wsign-conversion]
  108 |       jacobian_coo_cols_   = std::make_unique<IdxT[]>(nnz_);
      |                              ~~~                      ^~~~
/Users/55y/src/gridkit/GridKit/GridKit/Model/PowerElectronics/CircuitComponent.hpp:109:56: warning: implicit conversion changes signedness: 'long' to 'size_t' (aka 'unsigned long') [-Wsign-conversion]
  109 |       jacobian_coo_values_ = std::make_unique<RealT[]>(nnz_);
      |                              ~~~                       ^~~~

Comment thread GridKit/Model/PowerElectronics/MicrogridLine/MicrogridLine.cpp
@pelesh pelesh requested a review from PhilipFackler April 6, 2026 20:54
Copy link
Copy Markdown
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.

Great job!

@pelesh pelesh merged commit 21d0a2e into develop Apr 7, 2026
6 checks passed
@alexander-novo alexander-novo deleted the alex/remove-coo branch April 7, 2026 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove COO_Matrix from PowerElectronics module

2 participants