Skip to content

Conversation

@pelesh
Copy link
Collaborator

@pelesh pelesh commented Jul 16, 2025

Description

Default GridKit build was with SUNDIALS as dependency. This PR changes that to a build with no dependencies. This can help, for example, work on component models and unit tests, which do not require a solver enabled.

Proposed changes

Made changes to CMake so that GridKit modules and examples are built only if their required dependencies are enabled.

Checklist

  • All tests pass.
  • Code compiles cleanly with flags -Wall -Wpedantic -Wconversion -Wextra.
  • The new code follows GridKit™ style guidelines.
  • N/A There are unit tests for the new code.
  • N/A The new code is documented.
  • The feature branch is rebased with respect to the target branch.

Further comments

@pelesh pelesh requested review from lukelowry and nkoukpaizan July 16, 2025 03:16
@pelesh pelesh self-assigned this Jul 16, 2025
@pelesh pelesh added development Features/Tools related to development of GridKit, rather than use as a library. cmake labels Jul 16, 2025
Copy link
Collaborator

@lukelowry lukelowry left a comment

Choose a reason for hiding this comment

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

I agree with need of this PR. One minor issue, details below. Otherwise, builds correctly and all tests passed.

@@ -1,5 +1,5 @@
add_executable(test_case_format runCaseFormatTests.cpp)
target_include_directories(test_case_format PRIVATE ${THIRD_PARTY_DIR}/nlohmann-json/single_include ${THIRD_PARTY_DIR}/magic-enum/include)
target_include_directories(test_case_format PRIVATE ${GRIDKIT_THIRD_PARTY_DIR}/nlohmann-json/single_include ${GRIDKIT_THIRD_PARTY_DIR}/magic-enum/include)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I had to comment out this file for my machine to build. No issues with the json include, but compiler was unable to locate <magic_enum/magic_enum.hpp>. Could be something on my end, but unclear how to fix.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you try git submodule update --init?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Forgot to thank you for this fix. This was the issue. Is it possible to add this advice to the main README or perhaps a debug page for GridKit? @nkoukpaizan @pelesh

@nkoukpaizan
Copy link
Collaborator

It would be good to add GRIDKIT_ENABLE_ENZYME/GRIDKIT_USE_ENZYME to Definitions.hpp here.

And I understand why the other options are there (e.g., GRIDKIT_USE_RAJA), but my preference would be to only list options we currently support.

@pelesh
Copy link
Collaborator Author

pelesh commented Jul 16, 2025

It would be good to add GRIDKIT_ENABLE_ENZYME/GRIDKIT_USE_ENZYME to Definitions.hpp here.

And I understand why the other options are there (e.g., GRIDKIT_USE_RAJA), but my preference would be to only list options we currently support.

Addressed in e0e25a7.

@pelesh pelesh merged commit 5bf5d1d into develop Jul 16, 2025
4 checks passed
WiktoriaZielinskaORNL pushed a commit that referenced this pull request Jul 17, 2025
* Using SUNDIALS is off by default

* Make SUNDIALS and KLU optional.

* KLU is not a direct dependency anymore
@pelesh pelesh deleted the slaven/cmake_dependencies_off branch July 24, 2025 20:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cmake development Features/Tools related to development of GridKit, rather than use as a library.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants