Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide a way to disable Serac benchmarks #1292

Merged

Conversation

chapman39
Copy link
Collaborator

@tupek2 is adding Smith benchmarks. Smith needs a way to disable Serac benchmarks while running Smith's.

Serac benchmarks should not be run within Smith, since because Smith doesn't update Serac very often, so they won't be as accurate.

Eventually we should look into better cmake options #1282, but for now cmake_dependent_option should be re-added, imo.

@chapman39 chapman39 added the build Build system label Dec 11, 2024
@chapman39 chapman39 self-assigned this Dec 11, 2024
@tupek2
Copy link
Collaborator

tupek2 commented Dec 12, 2024

This seems reasonable to me. There is a clear pattern. Ideally, all our options can be like this, so I don't even have to specify project specific ones (DENABLE_CODEVELOP, for example doesnt exist, so we have to do it per project).

@tupek2
Copy link
Collaborator

tupek2 commented Dec 12, 2024

Thinking about this a bit... I think people get confused because
ENABLE_SERAC_BENCHMARKS=ON does not turn on ENABLE_BENCHMARKS.
ENABLE_BENCHMARKS is like the "house master value" in this design. It must be turned on for the local values to turn on/off.
We can either flip the logic, so ENABLE_BENCHMARKS just changes the local defaults, which can then always be turned on/off independently. Or we can keep the current design, but if we do, it might make it clearer if we change the local names to:
DISABLE_SERAC_BENCHMARKS=ON (all these disables would default to false) to limit the confusion potential a bit.

Then its clearer that ENABLE_BENCHMARKS turns them on for all projects, and per project we can disable them if needed.

@chapman39
Copy link
Collaborator Author

DENABLE_CODEVELOP, for example doesnt exist, so we have to do it per project

this variable doesn't exist, because it's not an option in BLT. here is a list of all blt options https://github.com/LLNL/blt/blob/develop/cmake/BLTOptions.cmake

@chapman39 chapman39 enabled auto-merge December 16, 2024 21:29
@chapman39 chapman39 merged commit 683ede4 into develop Dec 17, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Build system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants