Skip to content

LoadZIP Jacobians for new formulations and tests#364

Merged
abirchfield merged 16 commits intoabirchfield/zip_loadfrom
nicholson/zip_load
Apr 8, 2026
Merged

LoadZIP Jacobians for new formulations and tests#364
abirchfield merged 16 commits intoabirchfield/zip_loadfrom
nicholson/zip_load

Conversation

@nkoukpaizan
Copy link
Copy Markdown
Collaborator

@nkoukpaizan nkoukpaizan commented Apr 7, 2026

Description

This updates the Jacobian terms computed for LoadZIP and adds Jacobian tests.

Proposed changes

  • Replaced the computation of $\frac{\partial h}{\partial w_b}$ with $\frac{\partial h}{\partial y}$, $\frac{\partial f}{\partial y}$ and $\frac{\partial f}{\partial w_b}$ for the modified model implementation
  • Added a Jacobian test comparing Enzyme Jacobians and DependencyTracking Jacobians
  • Added an initialization for internal variables
  • Bug fix for the GovernorTest, where the Jacobian entries were not deduplicated

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.
  • [N/A] 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

  • The residual test fails after activating the LoadZIPTests. Either the model implementation or the answer keys are incorrect. @abirchfield Please check.
  • The $\frac{\partial f}{\partial y}$ is failing to build. This is because the internal variables are all algebraic, so $\frac{\partial f}{\partial y_p}$ is null. I need to implement a workaround for this. Implemented a workaround that I don't particularly like. We can discuss more offline.

@nkoukpaizan nkoukpaizan self-assigned this Apr 7, 2026
@pelesh
Copy link
Copy Markdown
Collaborator

pelesh commented Apr 7, 2026

CC @PhilipFackler

@nkoukpaizan nkoukpaizan requested review from abirchfield and pelesh and removed request for abirchfield April 7, 2026 20:23
@nkoukpaizan nkoukpaizan marked this pull request as ready for review April 7, 2026 20:23
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.

I believe residual tests were failing because of incorrect answer key. I updated test answers with correct numbers and pushed to this branch.

I also tweaked model parameters so that unit test answers are integer numbers for easier comparisons.

@abirchfield, please check and merge into your branch.

abirchfield pushed a commit that referenced this pull request Apr 8, 2026
* Alternate Jacobian terms for ZIPLoad.

* Jacobian test for LoadZIP.

* Fix phasor_load_zip test call.

* Minor formatting.

* LoadDataZIP --> LoadZIPData in CMakeLists.

* Comment out LoadZIP df/dy for now.

* Apply pre-commmit fixes

* Guard LoadZIPTests.jacobian test with ENABLE_ENZYME.

* Expect failure for LoadZIPTests.

* LoadZIP initialization.

* Fix typo.

* Simplify ifrac expression in LoadZIP.

* Activate df/dy term in LoadZIP with a hack for null df/dy'.

* Apply pre-commmit fixes

* Set correct answer key for LoadZIP test

* Apply pre-commmit fixes

---------

Co-authored-by: nkoukpaizan <[email protected]>
Co-authored-by: pelesh <[email protected]>
Co-authored-by: pelesh <[email protected]>
@nkoukpaizan nkoukpaizan deleted the nicholson/zip_load branch April 14, 2026 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants