Skip to content

Add new solver options to phasor dynamics application#483

Draft
Steven-Roberts wants to merge 5 commits into
developfrom
steven/new-json-solver-options
Draft

Add new solver options to phasor dynamics application#483
Steven-Roberts wants to merge 5 commits into
developfrom
steven/new-json-solver-options

Conversation

@Steven-Roberts

@Steven-Roberts Steven-Roberts commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Description

I think these options will be needed for the fixed versus adaptive time step comparisons.

Proposed changes

This adds JSON options for tolerances and a fixed time step

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

I noticed the existing dt JSON option has a few issues. It is documented as solver time step but it seems to be the output/monitoring time step. Perhaps it should be rename monitor_dt and the new fixed_dt should be called dt. Also with this rounding, the monitoring is not guaranteed to occur every dt simulation time.

int nout = static_cast<int>(std::round((event.time - curr_time) / dt));

Any suggestions on how to handle this @pelesh?

@nkoukpaizan nkoukpaizan added the enhancement New feature or request label Jul 10, 2026
@nkoukpaizan nkoukpaizan requested review from PhilipFackler, nkoukpaizan and pelesh and removed request for PhilipFackler July 10, 2026 14:04
@pelesh

pelesh commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Yes, dt is output monitoring step for adaptive step solvers.

@pelesh

pelesh commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Also with this rounding, the monitoring is not guaranteed to occur every dt simulation time.

I believe original idea was to have something like this:

for (i = 0; i < nout; ++i) {
  ...
  t += dt;
}

It was originally intended for nout to be an approximate number of outputs and output steps to be precisely of size dt.

@pelesh

pelesh commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

I like the idea of dt being the fixed time step and dt_moinitor to be output time step. Even if we use fixed time step, which is typically a quarter of a cycle, we probably don't want to output values at each step but say every 10th or so.

CC @lukelowry @abirchfield

@lukelowry

lukelowry commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

(related note) For validation, we need dt to be a quarter cycle so we have comparable samples against PowerWorld.

The large cases produce a lot of monitored output, @pelesh is right in his suggestion that dt_monitor ~10x the fixed time step dt for practical use. These are the same in existing solvers, so the distinction is usually moot. But we should make the distinction here

I like the naming dt_monitor, good suggestion. To prevent confusion, would dt_fixed be clearer than dt?

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.

4 participants