-
Notifications
You must be signed in to change notification settings - Fork 9
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
#2175: Add YAML as input as alternative to command-line arguments #2230
Conversation
Pipelines resultsPR tests (gcc-12, ubuntu, mpich) Build for 1863492 (2024-04-18 23:12:29 UTC)
PR tests (gcc-8, ubuntu, mpich, address sanitizer) Build for 531c3a8 (2024-07-17 20:10:33 UTC)
PR tests (clang-10, ubuntu, mpich) Build for 531c3a8 (2024-07-17 20:10:33 UTC)
PR tests (gcc-9, ubuntu, mpich, zoltan, json schema test) Build for 531c3a8 (2024-07-17 20:10:33 UTC)
PR tests (clang-9, ubuntu, mpich) Build for 531c3a8 (2024-07-17 20:10:33 UTC)
PR tests (gcc-10, ubuntu, openmpi, no LB) Build for 531c3a8 (2024-07-17 20:10:33 UTC)
PR tests (clang-11, ubuntu, mpich) Build for 531c3a8 (2024-07-17 20:10:33 UTC)
PR tests (clang-12, ubuntu, mpich) Build for 531c3a8 (2024-07-17 20:10:33 UTC)
PR tests (clang-13, ubuntu, mpich) Build for 531c3a8 (2024-07-17 20:10:33 UTC)
PR tests (clang-14, ubuntu, mpich) Build for 1863492 (2024-04-18 23:12:29 UTC)
PR tests (intel icpc, ubuntu, mpich) Build for 531c3a8 (2024-07-17 20:10:33 UTC)
PR tests (gcc-11, ubuntu, mpich, trace runtime, coverage) Build for 531c3a8 (2024-07-17 20:10:33 UTC)
PR tests (nvidia cuda 11.2, gcc-9, ubuntu, mpich) Build for 531c3a8 (2024-07-17 20:10:33 UTC)
PR tests (clang-13, alpine, mpich) Build for 531c3a8 (2024-07-17 20:10:33 UTC)
PR tests (intel icpx, ubuntu, mpich) Build for 1863492 (2024-04-18 23:12:29 UTC)
PR tests (nvidia cuda 12.2.0, gcc-9, ubuntu, mpich) Build for 1863492 (2024-04-18 23:12:29 UTC)
PR tests (gcc-12, ubuntu, mpich, verbose) Build for 8eda5a8 (2024-05-30 19:00:18 UTC)
PR tests (intel icpx, ubuntu, mpich, verbose) Build for 531c3a8 (2024-07-17 20:10:33 UTC)
PR tests (clang-14, ubuntu, mpich, verbose) Build for 531c3a8 (2024-07-17 20:10:33 UTC)
PR tests (nvidia cuda 12.2.0, gcc-9, ubuntu, mpich, verbose) Build for 531c3a8 (2024-07-17 20:10:33 UTC)
PR tests (gcc-12, ubuntu, mpich, verbose, kokkos) Build for 531c3a8 (2024-07-17 20:10:33 UTC)
|
4a97f5c
to
e2714fc
Compare
I'm not sure what's going on with the |
I think we should strip it down to bare minimum. We don't need Should we support using external |
I think this makes sense, especially since it seems the yaml-cpp source code doesn't pass the CI |
Also, is the use of a yaml-based config file incentivized by something (like Empire already using that file type)? If not, maybe we should consider using JSON for the config file as well? We already have it set up, and its structure is well-known to everyone. |
Yes, Empire is using YAML for their input and we want to be able to pass that directly to VT. LBAF and vt-tv are also using YAML so it seems that that route makes sense. |
I've taken the approach of this PR (using enum classes) in order to resolve the clang warnings. |
328fedf
to
889748e
Compare
0d5e7de
to
18792a9
Compare
3241ee7
to
cf76e82
Compare
@cwschilly Summary of our discussion:
|
…test for reading input yaml
9c6dcbe
to
531c3a8
Compare
Fixes #2175
Usage
Pass a yaml config file with
The order of precedence is currently
toml
orini
filemeaning that the command line arguments will overwrite everything (and a toml config will overwrite a yaml config).
Changes to
yaml-cpp
I had to make changes to the yaml-cpp library in order to avoid variable shadowing errors. I mostly used the solution from this PR, which added
enum class
where appropriate.In one case, I had to change a variable name from
YAML::Null
toYAML::BaseNull
(to avoid shadowingYAML::NodeType::Null
). I don't foresee this having consequences for our development (if we want to check if some input is null, we would useYAML::NodeType::Null
).I also made
yaml-cpp
a system include directory to silence all warnings (-Wtautological-constant-compare
was throwing an error on the intel icpx pipeline--this issue explains why it is not a meaningful warning and can be safely silenced).