Enzyme Jacobian for PhasorDynamics::Load within GridKit by nkoukpaizan · Pull Request #131 · ORNL/GridKit

nkoukpaizan · 2025-06-06T17:30:12Z

Description

Enzyme Jacobian for PhasorDynamics::Load within GridKit. This is done within the model library directly.

Proposed changes

Added a wrapper in src/AutomaticDifferentiation/Enzyme/ to compute the Jacobians. This requires the residual wrapper to be in global namespace, but other definitions can be inside the GridKit::Enzyme namespace.
Moved most of the implementation for the PhasorDynamics::Load into LoadImpl.hpp. This file in then included in Load.cpp (which calls Enzyme and provides a Jacobian) and LoadDependencyTracking.cpp, such that the two automatic differentiation approaches are compiled in different compilation units.
Added a enzyme_jacobian unit test in LoadTest.hpp.
Made the EnzymeVector tests more interesting to yield a lower triangular Jacobian rather than a diagonal one.

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.

Further comments

Second part of #124, after #126.

Draft, because an improved way to compile and link with Enzyme is needed before merging. I am open to suggestions. Maybe register the pass as @ashermancinelli [suggested earlier?](https://github.com//pull/29#discussion_r1893111882) - `ClangEnzymeFlags` (which is easy to link to) does not include all needed flags, e.g., `enzyme-auto-sparsity=1`. It works for the load, but will not work in general. The change to the `EnzymeVector` tests was to make sure I catch this. - I was able to change the `enzyme_add_executable` CMake macro to generate object files and libraries. This works in the `examples`, where we are linking in other GridKit libraries explicitly, but some includes are missing when I try to do this within GridKit itself.

Adding the desired flags (e.g., enzyme-auto-sparsity=1) to ClangEnzymeFlags with target_compile_options is a functional workaround.

pelesh · 2025-06-06T19:08:04Z

CC @sklus

pelesh · 2025-06-10T15:08:54Z

CMakeLists.txt

+  # todo Add a centralized configuration file
+  add_definitions(-DGRIDKIT_ENABLE_ENZYME)


I am not sure what is the purpose of this definition.

Using it to guard the Jacobian test. See

GridKit/tests/UnitTests/PhasorDynamics/LoadTests.hpp

Line 103 in aa9e7d6

#ifdef GRIDKIT_ENABLE_ENZYME

.

pelesh

This is great work!

There is one test failing on MacOS with Clang 16:

Details

Test project /Users/55y/src/gridkit/build-clang
    Start 21: EnzymePowerElectronicsCheck
1/1 Test #21: EnzymePowerElectronicsCheck ......***Failed    0.01 sec
Result incorrect at line = 10, column = 11, obtained = -2.06027e-14, reference = 0, difference = 2.06027e-14
Result incorrect at line = 11, column = 10, obtained = 2.06027e-14, reference = -0, difference = 2.06027e-14
Dense matrix: Autodiff Jacobian
-1 -0 -0 -0 -9.4e-05 -0 -0 -0 -0 -0 -0 -0 -0 -0 -0 -0 
0 0 0 0 0 0 0 0 0 0 0 0 0 0 1 0 
0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 1 
-1 -0 -0 -0 -9.4e-05 -0 -0 -0 -0 -0 -0 -0 -0 -0 -0 -0 
0 0 0 0 -31.41 0 0 0 0 0 0 0 0 0 0 0 
0 0 0 0 0 -31.41 0 0 0 0 0 0 0 0 0 0 
-0 -0 -0 -0 -0 -0.0013 -0 -0 -0 -0 -0 -0 -1 -0 -0 -0 
-0 -0 -0 -0 -0 -0 -0 -0 -0 -0 -0 -0 -0 -1 -0 -0 
0 0 0 0 0 -0.00013 420 0 0 0 -1 0 -0.1 -0.015708 0.75 0 
0 0 0 0 0 0 0 420 0 0 0 -1 0.015708 -0.1 0 0.75 
0 0 0 0 0 -1.44444 4.66667e+06 0 1.48148e+07 0 -11185.2 2.06027e-14 -1851.85 -174.533 8333.33 0 
0 0 0 0 0 0 0 4.66667e+06 0 1.48148e+07 -2.06027e-14 -11185.2 174.533 -1851.85 0 8333.33 
0 0 0 0 0 0 0 0 0 0 20000 0 0 314.159 -20000 0 
-0 -0 -0 -0 -0 -0 -0 -0 -0 -0 -0 20000 -314.159 -0 -0 -20000 
0 -2857.14 0 0 0 0 0 0 0 0 0 0 2857.14 0 -85.7143 314.159 
0 0 -2857.14 0 0 0 0 0 0 0 0 0 0 2857.14 -314.159 -85.7143 
Dense matrix: Reference Jacobian
-1 0 0 0 -9.4e-05 0 0 0 0 0 0 0 0 0 0 0 
0 0 0 -0 0 0 0 0 0 0 0 0 0 0 1 -0 
0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 1 
-1 0 0 0 -9.4e-05 0 0 0 0 0 0 0 0 0 0 0 
0 0 0 0 -31.41 0 0 0 0 0 0 0 0 0 0 0 
0 0 0 0 0 -31.41 0 0 0 0 0 0 -0 0 0 -0 
0 0 0 0 0 -0.0013 0 0 0 0 0 0 -1 0 0 0 
0 0 0 0 0 0 0 0 0 0 0 0 0 -1 0 0 
0 0 0 0 0 -0.00013 420 0 0 0 -1 0 -0.1 -0.015708 0.75 0 
0 0 0 0 0 0 0 420 0 0 0 -1 0.015708 -0.1 0 0.75 
0 0 0 0 -0 -1.44444 4.66667e+06 0 1.48148e+07 0 -11185.2 -0 -1851.85 -174.533 8333.33 0 
0 0 0 0 0 0 0 4.66667e+06 0 1.48148e+07 0 -11185.2 174.533 -1851.85 0 8333.33 
0 0 0 0 -0 0 0 0 0 0 20000 0 0 314.159 -20000 0 
0 0 0 0 0 0 0 0 0 0 0 20000 -314.159 0 0 -20000 
0 -2857.14 -0 0 -0 0 0 0 0 0 0 0 2857.14 0 -85.7143 314.159 
0 0 -2857.14 0 0 0 0 0 0 0 0 0 0 2857.14 -314.159 -85.7143 
Status: 2


0% tests passed, 1 tests failed out of 1

Total Test time (real) =   0.02 sec

The following tests FAILED:
         21 - EnzymePowerElectronicsCheck (Failed)
Errors while running CTest

I am not sure what is causing it. Needs to be fixed before the PR is merged.

examples/Enzyme/PowerElectronics/main.cpp

examples/Enzyme/Standalone/EnzymeSparse.cpp

src/AutomaticDifferentiation/Enzyme/SparseWrapper.hpp

…ary macro.

…y flags in examples.

…nzymePowerElectronicsCheck.

…ecutable CMake macro.

superwhiskers · 2025-06-10T17:27:41Z

@pelesh i'm not reproducing this error. has it already been fixed? clang 20 / arch linux / enzyme 0.0.182

pelesh · 2025-06-10T17:40:40Z

@pelesh i'm not reproducing this error. has it already been fixed? clang 20 / arch linux / enzyme 0.0.182

It is still there with clang 16.0.6 from Macports and enzyme 0.0.180. Perhaps this really requires the very latest Enzyme?

@nkoukpaizan, which Enzyme version do you use?

pelesh · 2025-06-10T17:50:33Z

@pelesh i'm not reproducing this error. has it already been fixed? clang 20 / arch linux / enzyme 0.0.182

I upgraded to Enzyme 0.0.182 but the same error persists. :/

nkoukpaizan · 2025-06-10T18:02:23Z

@pelesh i'm not reproducing this error. has it already been fixed? clang 20 / arch linux / enzyme 0.0.182

It is still there with clang 16.0.6 from Macports and enzyme 0.0.180. Perhaps this really requires the very latest Enzyme?

@nkoukpaizan, which Enzyme version do you use?

I am still at v0.0.173 on Frontier.

nkoukpaizan · 2025-06-10T18:03:37Z

@pelesh i'm not reproducing this error. has it already been fixed? clang 20 / arch linux / enzyme 0.0.182

I have not done anything to fix it. Trying to setup my dependencies on MacOS to reproduce.

superwhiskers

just some minor comments on ways to make things more concise if that's something you'd want to do. i'll leave actual approval to @pelesh

superwhiskers · 2025-06-10T19:46:46Z

examples/Enzyme/Library/Vector/VectorModel.cpp

+    for (size_t idx = 0; idx < n; ++idx)
    {
      v[idx] = 0.0;
    }


are we not able to use std::fill to replace this loop? this just appears to be a zero-fill of the vector.

examples/Enzyme/Standalone/EnzymeSparse.cpp

superwhiskers · 2025-06-10T19:53:42Z

examples/Enzyme/Standalone/EnzymeVector.cpp

+    for (size_t idx = 0; idx < x.size(); ++idx)
    {
      v[idx] = 0.0;
    }


same zero-fill comment above. std::fill could be used here.

examples/Enzyme/Standalone/EnzymeVector.cpp

pelesh · 2025-06-11T19:50:23Z

@pelesh i'm not reproducing this error. has it already been fixed? clang 20 / arch linux / enzyme 0.0.182

I can now confirm that EnzymePowerElectronicsCheck test failure occurs on arm arch only. I cannot reproduce it on Intel i7 with MacOS /clang 16.0.6/enzyme 0.0.173.

pelesh

The failing test is specific to the arm architecture and may be related to an issue in Enzyme itself. That issue is tracked in #137; it should not delay merging this PR.

* Separated out Enzyme macros from FindEnzyme and added a EnzymeAddLibrary macro. * Enzyme wrapper within GridKit. * Fixes to rebase. * More interesting vector residual for Enzyme examples. * Fix some warnings. * Address one more warning. * Apply pre-commmit fixes * Empty commit to trigger CI after pre-commit fixes. * Successful use of target_compiile_options to pass enzyme-auto-sparsity flags in examples. * More flags needed to get the correct answer to machine precision in EnzymePowerElectronicsCheck. * EnzymeAddLibrary not currently needed. * Apply pre-commmit fixes * Empty commit to trigger CI after pre-commit fixes. * Updated documentation in EnzymeAddLibrary. * Fix warnings previously hidden by manual builds through enzyme_add_executable CMake macro. * Use GridKit::Testing::isEqual in Enzyme examples. * Fixes to rebase. * Apply pre-commmit fixes * Empty commit to trigger CI after pre-commit fixes. * residual_wrapper can now be placed within GridKit::Enzyme namespace. --------- Co-authored-by: nkoukpaizan <nkoukpaizan@users.noreply.github.com>

nkoukpaizan self-assigned this Jun 6, 2025

nkoukpaizan added help wanted Extra attention is needed development Features/Tools related to development of GridKit, rather than use as a library. automatic differentiation labels Jun 6, 2025

nkoukpaizan changed the title ~~Nicholson/autodiff retake~~ Enzyme Jacobian for PhasorDynamics::Load within GridKit Jun 6, 2025

nkoukpaizan requested review from alexander-novo, pelesh and superwhiskers June 6, 2025 17:40

nkoukpaizan marked this pull request as ready for review June 7, 2025 16:30

pelesh reviewed Jun 10, 2025

View reviewed changes

pelesh requested changes Jun 10, 2025

View reviewed changes

examples/Enzyme/PowerElectronics/main.cpp Outdated Show resolved Hide resolved

examples/Enzyme/Standalone/EnzymeSparse.cpp Show resolved Hide resolved

src/AutomaticDifferentiation/Enzyme/SparseWrapper.hpp Show resolved Hide resolved

nkoukpaizan and others added 17 commits June 10, 2025 12:33

Separated out Enzyme macros from FindEnzyme and added a EnzymeAddLibr…

76cb30a

…ary macro.

Enzyme wrapper within GridKit.

853f3c0

Fixes to rebase.

d2c3f8a

More interesting vector residual for Enzyme examples.

b3810bd

Fix some warnings.

af9cef7

Address one more warning.

7878bc6

Apply pre-commmit fixes

58d7ffa

Empty commit to trigger CI after pre-commit fixes.

f342e8c

Successful use of target_compiile_options to pass enzyme-auto-sparsit…

75aa4d5

…y flags in examples.

More flags needed to get the correct answer to machine precision in E…

b21eccc

…nzymePowerElectronicsCheck.

EnzymeAddLibrary not currently needed.

4ea98c3

Apply pre-commmit fixes

1679497

Empty commit to trigger CI after pre-commit fixes.

97afa84

Updated documentation in EnzymeAddLibrary.

6c724b3

Fix warnings previously hidden by manual builds through enzyme_add_ex…

c32524f

…ecutable CMake macro.

Use GridKit::Testing::isEqual in Enzyme examples.

0c479c3

Fixes to rebase.

88b4153

nkoukpaizan force-pushed the nicholson/autodiff-retake branch from f856ceb to 88b4153 Compare June 10, 2025 16:43

nkoukpaizan and others added 2 commits June 10, 2025 16:43

Apply pre-commmit fixes

03b165d

Empty commit to trigger CI after pre-commit fixes.

c5c5203

superwhiskers reviewed Jun 10, 2025

View reviewed changes

residual_wrapper can now be placed within GridKit::Enzyme namespace.

9261d2c

pelesh approved these changes Jun 11, 2025

View reviewed changes

nkoukpaizan merged commit 9b92afb into develop Jun 12, 2025
4 checks passed

nkoukpaizan deleted the nicholson/autodiff-retake branch June 30, 2025 15:08

		# todo Add a centralized configuration file
		add_definitions(-DGRIDKIT_ENABLE_ENZYME)

Conversation

nkoukpaizan commented Jun 6, 2025 • edited by pelesh Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Proposed changes

Checklist

Further comments

Uh oh!

pelesh commented Jun 6, 2025

Uh oh!

pelesh Jun 10, 2025

Choose a reason for hiding this comment

Uh oh!

nkoukpaizan Jun 10, 2025

Choose a reason for hiding this comment

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!

Uh oh!

Uh oh!

Uh oh!

superwhiskers commented Jun 10, 2025

Uh oh!

pelesh commented Jun 10, 2025

Uh oh!

pelesh commented Jun 10, 2025

Uh oh!

nkoukpaizan commented Jun 10, 2025

Uh oh!

nkoukpaizan commented Jun 10, 2025

Uh oh!

superwhiskers left a comment

Choose a reason for hiding this comment

Uh oh!

superwhiskers Jun 10, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

superwhiskers Jun 10, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pelesh commented Jun 11, 2025

Uh oh!

pelesh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

nkoukpaizan commented Jun 6, 2025 •

edited by pelesh

Loading

pelesh left a comment •

edited

Loading