Skip to content

Remove data copying for internal component variables, derivatives, and residuals in PowerElectronics#379

Draft
alexander-novo wants to merge 8 commits intodevelopfrom
alex/pow-elec-remove-data-copy
Draft

Remove data copying for internal component variables, derivatives, and residuals in PowerElectronics#379
alexander-novo wants to merge 8 commits intodevelopfrom
alex/pow-elec-remove-data-copy

Conversation

@alexander-novo
Copy link
Copy Markdown
Collaborator

Description

Partially addresses #353.

Removes data copying between the system and components for internal variables, derivatives, and residuals when evaluating residuals and Jacobians.

Proposed changes

CircuitComponent now has three pointers:

  • int_: a pointer to the component's internal variables in the system vector
  • intp_: a pointer to the component's internal derivatives in the system vector
  • int_f_: a pointer to the component's internal residuals in the system vector

PowerElectronicsModel::distributeVectors() and PowerElectronicsModel::evaluateResidual() have been modified to only copy variables between y_ and f_ if they are external. Therefore, components must use the three new pointers above to correctly calculate residuals and 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

This solution won't work quite right for system objects which are components in another system. Maybe we want some sort of flag that checks for this situation and doesn't allocate a y_ or f_ vector?

@alexander-novo alexander-novo requested a review from pelesh April 23, 2026 14:23
@alexander-novo
Copy link
Copy Markdown
Collaborator Author

@pelesh I'm not really sure what to do about the enzyme example, since the DG model's residual has diverged from what's given in that example. Let me know what you think is best.

@pelesh pelesh marked this pull request as draft April 27, 2026 15:36
@pelesh
Copy link
Copy Markdown
Collaborator

pelesh commented Apr 27, 2026

@pelesh I'm not really sure what to do about the enzyme example, since the DG model's residual has diverged from what's given in that example. Let me know what you think is best.

Let's merge the other power electronics PRs, first.

My understanding is that only variables were reordered and equations remained the same. In that case, it should not be difficult to fix the answer key for the Enzyme test.

@pelesh
Copy link
Copy Markdown
Collaborator

pelesh commented Apr 27, 2026

I set this to draft for now until we merge other related PRs.

@alexander-novo alexander-novo force-pushed the alex/power-elec-dg-reorder branch from db31722 to 491b7ca Compare April 27, 2026 19:48
@alexander-novo alexander-novo force-pushed the alex/pow-elec-remove-data-copy branch 3 times, most recently from 8773a38 to 30704e2 Compare April 30, 2026 14:33
@alexander-novo
Copy link
Copy Markdown
Collaborator Author

@pelesh I'm not sure the DistributedGeneratorTest makes sense. The test expects to evaluate the residual of a reference DG where the delta variable is non-zero, which is now no longer possible and I don't think should be a supported feature of the model?

@alexander-novo
Copy link
Copy Markdown
Collaborator Author

Well I just changed it to be a test of a non-reference generator, and edited the answer key to match.

@alexander-novo alexander-novo force-pushed the alex/power-elec-dg-reorder branch 2 times, most recently from d87c9b3 to 77be7b9 Compare April 30, 2026 20:52
@alexander-novo alexander-novo force-pushed the alex/pow-elec-remove-data-copy branch from bae5286 to 60c0212 Compare April 30, 2026 20:56
@alexander-novo alexander-novo force-pushed the alex/power-elec-dg-reorder branch from 77be7b9 to 6e597c3 Compare April 30, 2026 21:08
@alexander-novo alexander-novo changed the base branch from alex/power-elec-dg-reorder to develop May 1, 2026 13:58
Previous test was testing a scenario with a refrence DG where delta != 0, which is no longer possible. New test does a non-reference DG, and the answer key has been updated with the expected value of the first residual (0) for a non-reference DG.
@alexander-novo alexander-novo force-pushed the alex/pow-elec-remove-data-copy branch from 60c0212 to 4f84918 Compare May 1, 2026 14:00
@pelesh
Copy link
Copy Markdown
Collaborator

pelesh commented May 1, 2026

This is very nice work for what I could see thus far. I would only avoid calling internal variables int_ becayuse in C-speak "int" is widely recognized as "integer" and int_ is too close for comfort to the keyword int.

How about y_int_?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants