Skip to content

A step toward generalizing the phasor dynamics examples.#338

Open
PhilipFackler wants to merge 3 commits intodevelopfrom
PhilipFackler/update-threebusbasic
Open

A step toward generalizing the phasor dynamics examples.#338
PhilipFackler wants to merge 3 commits intodevelopfrom
PhilipFackler/update-threebusbasic

Conversation

@PhilipFackler
Copy link
Collaborator

Description

This updates the ThreeBusBasicJson example to compare the output of the monitored variables with the reference solution (rather that the result of the custom callback). I created a reference solution from the original ThreeBusBasic.hpp file and stored it as ThreeBusBasic.ref.csv.

Closes #337

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

In this iteration, the code depends on a hard-coded file name. It expects the monitored variable output to be stored in mon.csv. This is specified in the example json file so it makes sense from the perspective that this is still a specific example with its own source code.

As we move toward a generalized application for these, we can have a command-line option specifying which output files to compare (optionally, of course, since we may not be testing the output).

@PhilipFackler PhilipFackler requested a review from pelesh February 13, 2026 17:33
@PhilipFackler PhilipFackler force-pushed the PhilipFackler/update-threebusbasic branch from a3f8024 to c8f2e0d Compare February 16, 2026 18:49
@pelesh pelesh marked this pull request as ready for review February 17, 2026 02:46
Copy link
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.

  • Using CSV files as reference and output data is good!
  • 3-bus example is much cleaner and can be used as a template for refactoring other examples.
  • Utilities for processing and comparing data in CSV files simplify examples implementation.

I suggest @abirchfield and @lukelowry review and provide feedback before it is merged.

The code fails to build on my machine. Make sure tests pass in CI pipeline.

@pelesh
Copy link
Collaborator

pelesh commented Feb 17, 2026

I am getting linker error when linking Logger with file utilities:

Details
[ 58%] Linking CXX executable adjoint
Undefined symbols for architecture arm64:
  "GridKit::Utilities::Logger::error()", referenced from:
      GridKit::Testing::checkOpenFile(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>> const&) in runCliOptionsTests.cpp.o
      GridKit::Testing::checkOpenFile(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>> const&) in runCliOptionsTests.cpp.o
      GridKit::Testing::compareCSV(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>> const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>> const&) in runCliOptionsTests.cpp.o
      GridKit::Testing::compareCSV(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>> const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>> const&) in runCliOptionsTests.cpp.o
ld: symbol(s) not found for architecture arm64
clang++: error: linker command failed with exit code 1 (use -v to see invocation)
make[2]: *** [tests/UnitTests/Utilities/test_cli_options] Error 1
make[1]: *** [tests/UnitTests/Utilities/CMakeFiles/test_cli_options.dir/all] Error 2
make[1]: *** Waiting for unfinished jobs....
Undefined symbols for architecture arm64:
  "GridKit::Utilities::Logger::error()", referenced from:
      GridKit::Testing::checkOpenFile(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>> const&) in runSparseTests.cpp.o
      GridKit::Testing::checkOpenFile(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>> const&) in runSparseTests.cpp.o
      GridKit::Testing::compareCSV(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>> const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>> const&) in runSparseTests.cpp.o
      GridKit::Testing::compareCSV(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>> const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>> const&) in runSparseTests.cpp.o
ld: symbol(s) not found for architecture arm64
clang++: error: linker command failed with exit code 1 (use -v to see invocation)
Undefined symbols for architecture arm64:
  "GridKit::Utilities::Logger::error()", referenced from:
      GridKit::Testing::checkOpenFile(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>> const&) in runCircuitNodeTests.cpp.o
      GridKit::Testing::checkOpenFile(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>> const&) in runCircuitNodeTests.cpp.o
      GridKit::Testing::compareCSV(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>> const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>> const&) in runCircuitNodeTests.cpp.o
      GridKit::Testing::compareCSV(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>> const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>> const&) in runCircuitNodeTests.cpp.o
ld: symbol(s) not found for architecture arm64
make[2]: *** [tests/UnitTests/LinearAlgebra/SparseMatrix/test_sparse] Error 1
make[1]: *** [tests/UnitTests/LinearAlgebra/SparseMatrix/CMakeFiles/test_sparse.dir/all] Error 2
clang++: error: linker command failed with exit code 1 (use -v to see invocation)
make[2]: *** [tests/UnitTests/PowerElectronics/test_power_electronics_node] Error 1
make[1]: *** [tests/UnitTests/PowerElectronics/CMakeFiles/test_power_electronics_node.dir/all] Error 2

I suggest implementing file utilities in a source file, building it as a standalone object and linking it to logger. It looks as if some of the include directives include code that needs to be linked to GridKit::utilities_logger object, but that is not obvious from the example application itself.

@pelesh
Copy link
Collaborator

pelesh commented Feb 17, 2026

Once I pass the building woes, the result verification in simplified examples fails:

94% tests passed, 3 tests failed out of 54

Total Test time (real) =  16.25 sec

The following tests FAILED:
         24 - ThreeBusBasicJson (Failed)
         25 - ThreeBusBasicJson_no_arg (Failed)
         40 - EnzymePowerElectronicsCheck (Failed)
Errors while running CTest

@PhilipFackler PhilipFackler force-pushed the PhilipFackler/update-threebusbasic branch 2 times, most recently from 699f389 to ea4e668 Compare February 17, 2026 21:23
@PhilipFackler PhilipFackler force-pushed the PhilipFackler/update-threebusbasic branch from ea4e668 to a7b3408 Compare February 17, 2026 21:31
@pelesh
Copy link
Collaborator

pelesh commented Feb 17, 2026

I am not sure how the following test passes:

53: --- PASS: Test jacobian
53: [ERROR] Genrou: pmech signal attached with no linked governor
53: [ERROR] Genrou: efd signal attached with no linked exciter
53: [ERROR] Genrou: pmech signal attached with no linked governor
53: [ERROR] Genrou: efd signal attached with no linked exciter
53: [ERROR] Component errors: 4

We are either signaling error where there is none or we are writing/reading from unallocated address and getting away with it. This seems to be out of scope of this PR, though.

CC @nkoukpaizan

Comment on lines 292 to +295
monitor_->set(Variable::omega, [this]
{ return y_[1]; });
monitor_->set(Variable::speed, [this]
{ return 1.0 + y_[1]; });
Copy link
Collaborator

Choose a reason for hiding this comment

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

@abirchfield, do we need outputs for omega (deviation from nominal speed) and speed?

@nkoukpaizan
Copy link
Collaborator

I am not sure how the following test passes:

53: --- PASS: Test jacobian
53: [ERROR] Genrou: pmech signal attached with no linked governor
53: [ERROR] Genrou: efd signal attached with no linked exciter
53: [ERROR] Genrou: pmech signal attached with no linked governor
53: [ERROR] Genrou: efd signal attached with no linked exciter
53: [ERROR] Component errors: 4

We are either signaling error where there is none or we are writing/reading from unallocated address and getting away with it. This seems to be out of scope of this PR, though.

CC @nkoukpaizan

I'll let @PhilipFackler chime in, but looks like that is the expected behavior of the signalError test.

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.

Create an example demonstrating monitored variables usage

3 participants