Skip to content

Cleanup Examples - #577

Open
lukelowry wants to merge 14 commits into
developfrom
lukel/example-cleanup-dev
Open

lukelowry wants to merge 14 commits into
developfrom
lukel/example-cleanup-dev

Conversation

@lukelowry

@lukelowry lukelowry commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator

Description

Remove duplicate case data and organize the PhasorDynamics examples by purpose. Simulation examples now live under Simulation/, alongside the existing Validation/ studies, with reusable system models in cases/PhasorDynamics/.

Proposed changes

  • Replace the Tiny/Small/Medium/Large layout with Simulation/ , following the conventions of Validation/.
  • Move the TwoBus, ThreeBus, TenGen, TwoArea, and ACTIVSg10k examples into Simulation/.
  • Remove the legacy Illinois, Texas, WECC, and Hawaii example copies in favor of the shared cases and validation studies.
  • Preserve the NewEngland 39-fault contingency study under the integration tests.
  • Update CMake, consumer example paths, and integration test paths for the new layout.
  • Reorganize the documentation and add links between examples, validation studies, and reusable case descriptions.

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

Further comments

None

@lukelowry
lukelowry force-pushed the lukel/example-cleanup-dev branch 2 times, most recently from 778e4ef to b413c6c Compare September 13, 2026 01:29
@lukelowry

Copy link
Copy Markdown
Collaborator Author

@pelesh @nkoukpaizan

Highest value

@nkoukpaizan

Copy link
Copy Markdown
Collaborator

@lukelowry Could you expand on the proposed changes in the PR summary?

@lukelowry

Copy link
Copy Markdown
Collaborator Author

@nkoukpaizan should be good now!

@pelesh

pelesh commented Sep 15, 2026

Copy link
Copy Markdown
Collaborator

Why is New England contingency analysis case in tests directory? I think it is more fitting in examples.

@pelesh pelesh left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is much needed and welcome work. There are a few remaining issues to take care of:

  • I suggest to put all case files in directory cases. This includes all 3- and 2-bus cases we created.
  • All 3- and 2-bus cases that have reference solution should be moved to examples/PhasorDynamics/Validation directory and run as CMake tests.
  • There should be a README file(s) in examples/PhasorDynamics/Validation describing validation pass/fail criteria.
  • Examples that do not use GridKit apps should be turned into integration tests. As far as I can see, these include the two TenGen cases.
  • New England contingency analysis test in tests/IntegrationTests/PhasorDynamics/ContingencyAnalysis fits better in examples/PhasorDynamics/Validation directory.
  • Examples in examples/PhasorDynamics/Simulation are run as CMake tests but it is unclear what are the pass/fail criteria there. I would keep those examples but remove the CMake tests; running examples in validation subdirectory as CMake tests should be enough.

@lukelowry
lukelowry requested a review from pelesh September 15, 2026 21:48
@lukelowry

lukelowry commented Sep 15, 2026

Copy link
Copy Markdown
Collaborator Author

@pelesh I have applied your requested changes except for:

  • Validation criteria: I am not sure what you mean, can you clarify?
  • Why IEEE39 ContingencyAnalysis example in Validation? what is it validating against? The example is good, it is showing N-1 security for the case.

Its starting to look quite nice

@pelesh

pelesh commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator

@pelesh I have applied your requested changes except for:

  • Validation criteria: I am not sure what you mean, can you clarify?

There should be a description what simulation outputs are checked, what the reference (ground thruth) values are, what the tolerance is (i.e. what constitutes a valid result), etc. The user should have sufficient information to understand what checks are done during "validation".

@pelesh

pelesh commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator
  • Why IEEE39 ContingencyAnalysis example in Validation? what is it validating against? The example is good, it is showing N-1 security for the case.

My suggestion was more to move it from tests to examples. Since it was in tests I assumed it was checking results against some reference value. We can put it in examples/PhasorDynamics/ContingencyAnalysis directory.

@pelesh

pelesh commented Sep 17, 2026

Copy link
Copy Markdown
Collaborator

The two TenGen tests always return 0 and may produce false positives. Need to add a check if the results are correct and return pass/fail outcome.

@lukelowry
lukelowry force-pushed the lukel/example-cleanup-dev branch from ca3f9df to 8065d17 Compare September 17, 2026 19:43
@lukelowry
lukelowry force-pushed the lukel/example-cleanup-dev branch from 8065d17 to 87b5578 Compare September 17, 2026 20:22

@nkoukpaizan nkoukpaizan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of minor comments. Otherwise looks good!

Comment thread examples/PhasorDynamics/README.md Outdated
Comment thread examples/PhasorDynamics/Simulation/README.md Outdated
Comment thread examples/PhasorDynamics/README.md Outdated
@lukelowry

Copy link
Copy Markdown
Collaborator Author

@pelesh once the CI passes, all should be good. Addressed all reviewer suggestions

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