#2175: Add YAML as input as alternative to command-line arguments

cwschilly commented 9 months ago

Fixes #2175

Usage

Pass a yaml config file with

--vt_input_config_yaml /path/to/yaml/file

The order of precedence is currently

command line arguments
input toml or ini file
input yaml,

meaning 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 to YAML::BaseNull (to avoid shadowing YAML::NodeType::Null). I don't foresee this having consequences for our development (if we want to check if some input is null, we would use YAML::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).

github-actions[bot] commented 9 months ago

Pipelines results

PR tests (gcc-12, ubuntu, mpich)

Build for 1863492042030b8ed687c320ea4205e90941e282 (2024-04-18 23:12:29 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (gcc-8, ubuntu, mpich, address sanitizer)

Build for 531c3a8e1218de56c4d5086dea793f818e763a42 (2024-07-17 20:10:33 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (clang-10, ubuntu, mpich)

Build for 531c3a8e1218de56c4d5086dea793f818e763a42 (2024-07-17 20:10:33 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (gcc-9, ubuntu, mpich, zoltan, json schema test)

Build for 531c3a8e1218de56c4d5086dea793f818e763a42 (2024-07-17 20:10:33 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (clang-9, ubuntu, mpich)

Build for 531c3a8e1218de56c4d5086dea793f818e763a42 (2024-07-17 20:10:33 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (gcc-10, ubuntu, openmpi, no LB)

Build for 531c3a8e1218de56c4d5086dea793f818e763a42 (2024-07-17 20:10:33 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (clang-11, ubuntu, mpich)

Build for 531c3a8e1218de56c4d5086dea793f818e763a42 (2024-07-17 20:10:33 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (clang-12, ubuntu, mpich)

Build for 531c3a8e1218de56c4d5086dea793f818e763a42 (2024-07-17 20:10:33 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (clang-13, ubuntu, mpich)

Build for 531c3a8e1218de56c4d5086dea793f818e763a42 (2024-07-17 20:10:33 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (clang-14, ubuntu, mpich)

Build for 1863492042030b8ed687c320ea4205e90941e282 (2024-04-18 23:12:29 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (intel icpc, ubuntu, mpich)

Build for 531c3a8e1218de56c4d5086dea793f818e763a42 (2024-07-17 20:10:33 UTC)

remark #11074: Inlining inhibited by limit max-size 
remark #11074: Inlining inhibited by limit max-total-size 
remark #11076: To get full report use -qopt-report=4 -qopt-report-phase ipo
remark #11074: Inlining inhibited by limit max-size 
remark #11074: Inlining inhibited by limit max-total-size 
remark #11076: To get full report use -qopt-report=4 -qopt-report-phase ipo
remark #11074: Inlining inhibited by limit max-size 
remark #11074: Inlining inhibited by limit max-total-size 
remark #11076: To get full report use -qopt-report=4 -qopt-report-phase ipo
remark #11074: Inlining inhibited by limit max-size 
remark #11074: Inlining inhibited by limit max-total-size 
remark #11076: To get full report use -qopt-report=4 -qopt-report-phase ipo
remark #11074: Inlining inhibited by limit max-size 
remark #11074: Inlining inhibited by limit max-total-size 
remark #11076: To get full report use -qopt-report=4 -qopt-report-phase ipo
remark #11074: Inlining inhibited by limit max-size 
remark #11074: Inlining inhibited by limit max-total-size 
remark #11076: To get full report use -qopt-report=4 -qopt-report-phase ipo
remark #11074: Inlining inhibited by limit max-total-size 
remark #11076: To get full report use -qopt-report=4 -qopt-report-phase ipo
remark #11074: Inlining inhibited by limit max-size 
remark #11074: Inlining inhibited by limit max-total-size 
remark #11076: To get full report use -qopt-report=4 -qopt-report-phase ipo
remark #11074: Inlining inhibited by limit max-size 
remark #11074: Inlining inhibited by limit max-total-size 
remark #11076: To get full report use -qopt-report=4 -qopt-report-phase ipo
remark #11074: Inlining inhibited by limit max-size 
remark #11074: Inlining inhibited by limit max-total-size 
remark #11076: To get full report use -qopt-report=4 -qopt-report-phase ipo
remark #11074: Inlining inhibited by limit max-total-size 
remark #11076: To get full report use -qopt-report=4 -qopt-report-phase ipo
remark #11074: Inlining inhibited by limit max-total-size 
remark #11076: To get full report use -qopt-report=4 -qopt-report-phase ipo
remark #11074: Inlining inhibited by limit max-size 
remark #11074: Inlining inhibited by limit max-total-size 
remark #11076: To get full report use -qopt-report=4 -qopt-report-phase ipo
remark #11074: Inlining inhibited by limit max-size 
remark #11074: Inlining inhibited by limit max-total-size 
remark #11076: To get full report use -qopt-report=4 -qopt-report-phase ipo
remark #11074: Inlining inhibited by limit max-size 
remark #11074: Inlining inhibited by limit max-total-size 
remark #11076: To get full report use -qopt-report=4 -qopt-report-phase ipo
remark #11074: Inlining inhibited by limit max-size 
remark #11074: Inlining inhi%0D%0A%0D%0A%0D%0A ==> And there is more. Read log. <==

Build log

PR tests (gcc-11, ubuntu, mpich, trace runtime, coverage)

Build for 531c3a8e1218de56c4d5086dea793f818e763a42 (2024-07-17 20:10:33 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (nvidia cuda 11.2, gcc-9, ubuntu, mpich)

Build for 531c3a8e1218de56c4d5086dea793f818e763a42 (2024-07-17 20:10:33 UTC)

/vt/src/vt/pipe/pipe_manager.impl.h(135): warning: missing return statement at end of non-void function "vt::pipe::PipeManager::makeSend<f,Target>(Target) [with f=&vt::vrt::collection::lb::GreedyLB::collectHandler, Target=vt::objgroup::proxy::ProxyElm<vt::vrt::collection::lb::GreedyLB>]"
          detected during:
            instantiation of "auto vt::pipe::PipeManager::makeSend<f,Target>(Target) [with f=&vt::vrt::collection::lb::GreedyLB::collectHandler, Target=vt::objgroup::proxy::ProxyElm<vt::vrt::collection::lb::GreedyLB>]" 
/vt/src/vt/objgroup/proxy/proxy_objgroup.impl.h(221): here
            instantiation of "vt::objgroup::proxy::Proxy<ObjT>::PendingSendType vt::objgroup::proxy::Proxy<ObjT>::reduce<f,Op,Target,Args...>(Target, Args &&...) const [with ObjT=vt::vrt::collection::lb::GreedyLB, f=&vt::vrt::collection::lb::GreedyLB::collectHandler, Op=vt::collective::PlusOp, Target=vt::objgroup::proxy::ProxyElm<vt::vrt::collection::lb::GreedyLB>, Args=<vt::vrt::collection::lb::GreedyPayload>]" 
/vt/src/vt/vrt/collection/balance/greedylb/greedylb.cc(222): here

/vt/src/vt/pipe/pipe_manager.impl.h(135): warning: missing return statement at end of non-void function "vt::pipe::PipeManager::makeSend<f,Target>(Target) [with f=&MyObj::handler, Target=vt::objgroup::proxy::ProxyElm<MyObj>]"
          detected during instantiation of "auto vt::pipe::PipeManager::makeSend<f,Target>(Target) [with f=&MyObj::handler, Target=vt::objgroup::proxy::ProxyElm<MyObj>]" 
/vt/examples/callback/callback.cc(147): here

/vt/src/vt/pipe/pipe_manager.impl.h(135): warning: missing return statement at end of non-void function "vt::pipe::PipeManager::makeSend<f,Target>(Target) [with f=&colHan, Target=vt::vrt::collection::VrtElmProxy<MyCol, vt::Index1D>]"
          detected during instantiation of "auto vt::pipe::PipeManager::makeSend<f,Target>(Target) [with f=&colHan, Target=vt::vrt::collection::VrtElmProxy<MyCol, vt::Index1D>]" 
/vt/examples/callback/callback.cc(153): here

/vt/src/vt/pipe/pipe_manager.impl.h(135): warning: missing return statement at end of non-void function "vt::pipe::PipeManager::makeSend<f,Target>(Target) [with f=&MyObj::handler, Target=vt::objgroup::proxy::ProxyElm<MyObj>]"
          detected during instantiation of "auto vt::pipe::PipeManager::makeSend<f,Target>(Target) [with f=&MyObj::handler, Target=vt::objgroup::proxy::ProxyElm<MyObj>]" 
/vt/examples/callback/callback.cc(147): here

/vt/src/vt/pipe/pipe_manager.impl.h(135): warning: missing return statement at end of non-void function "vt::pipe::PipeManager::makeSend<f,Target>(Target) [with f=&colHan, Target=vt::vrt::collection::VrtElmProxy<MyCol, vt::Index1D>]"
          detected during instantiation of "auto vt::pipe::PipeManager::makeSend<f,Target>(Target) [with f=&colHan, Target=vt::vrt::collection::VrtElmProxy<MyCol, vt::Index1D>]" 
/vt/examples/callback/callback.cc(153%0D%0A%0D%0A%0D%0A ==> And there is more. Read log. <==

Build log

PR tests (clang-13, alpine, mpich)

Build for 531c3a8e1218de56c4d5086dea793f818e763a42 (2024-07-17 20:10:33 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (intel icpx, ubuntu, mpich)

Build for 1863492042030b8ed687c320ea4205e90941e282 (2024-04-18 23:12:29 UTC)

In file included from /vt/lib/yaml-cpp/src/convert.cpp:3:
/vt/lib/yaml-cpp/include/yaml-cpp/node/convert.h:103:7: warning: comparison with NaN always evaluates to false in fast floating point modes [-Wtautological-constant-compare]
  if (std::isnan(rhs)) {
      ^~~~~~~~~~~~~~~
/vt/lib/yaml-cpp/include/yaml-cpp/node/convert.h:212:1: note: in instantiation of function template specialization 'YAML::conversion::inner_encode<float>' requested here
YAML_DEFINE_CONVERT_STREAMABLE_SIGNED(float);
^
/vt/lib/yaml-cpp/include/yaml-cpp/node/convert.h:194:3: note: expanded from macro 'YAML_DEFINE_CONVERT_STREAMABLE_SIGNED'
  YAML_DEFINE_CONVERT_STREAMABLE(type, -)
  ^
/vt/lib/yaml-cpp/include/yaml-cpp/node/convert.h:155:19: note: expanded from macro 'YAML_DEFINE_CONVERT_STREAMABLE'
      conversion::inner_encode(rhs, stream);                               \n                  ^
/vt/lib/yaml-cpp/include/yaml-cpp/node/convert.h:105:14: warning: comparison with infinity always evaluates to false in fast floating point modes [-Wtautological-constant-compare]
  } else if (std::isinf(rhs)) {
             ^~~~~~~~~~~~~~~
/vt/lib/yaml-cpp/include/yaml-cpp/node/convert.h:103:7: warning: comparison with NaN always evaluates to false in fast floating point modes [-Wtautological-constant-compare]
  if (std::isnan(rhs)) {
      ^~~~~~~~~~~~~~~
/vt/lib/yaml-cpp/include/yaml-cpp/node/convert.h:213:1: note: in instantiation of function template specialization 'YAML::conversion::inner_encode<double>' requested here
YAML_DEFINE_CONVERT_STREAMABLE_SIGNED(double);
^
/vt/lib/yaml-cpp/include/yaml-cpp/node/convert.h:194:3: note: expanded from macro 'YAML_DEFINE_CONVERT_STREAMABLE_SIGNED'
  YAML_DEFINE_CONVERT_STREAMABLE(type, -)
  ^
/vt/lib/yaml-cpp/include/yaml-cpp/node/convert.h:155:19: note: expanded from macro 'YAML_DEFINE_CONVERT_STREAMABLE'
      conversion::inner_encode(rhs, stream);                               \n                  ^
/vt/lib/yaml-cpp/include/yaml-cpp/node/convert.h:105:14: warning: comparison with infinity always evaluates to false in fast floating point modes [-Wtautological-constant-compare]
  } else if (std::isinf(rhs)) {
             ^~~~~~~~~~~~~~~
/vt/lib/yaml-cpp/include/yaml-cpp/node/convert.h:103:7: warning: comparison with NaN always evaluates to false in fast floating point modes [-Wtautological-constant-compare]
  if (std::isnan(rhs)) {
      ^~~~~~~~~~~~~~~
/vt/lib/yaml-cpp/include/yaml-cpp/node/convert.h:214:1: note: in instantiation of function template specialization 'YAML::conversion::inner_encode<long double>' requested here
YAML_DEFINE_CONVERT_STREAMABLE_SIGNED(long double);
^
/vt/lib/yaml-cpp/include/yaml-cpp/node/convert.h:194:3: note: expanded from macro 'YAML_DEFINE_CONVERT_STREAMABLE_SIGNED'
  YAML_DEFINE_CON%0D%0A%0D%0A%0D%0A ==> And there is more. Read log. <==

Build log

PR tests (nvidia cuda 12.2.0, gcc-9, ubuntu, mpich)

Build for 1863492042030b8ed687c320ea4205e90941e282 (2024-04-18 23:12:29 UTC)

/vt/lib/CLI/CLI/CLI11.hpp(1029): warning #2361-D: invalid narrowing conversion from "double" to "unsigned long"
          TT { std::declval<CC>() }
               ^
          detected during:
            instantiation of "vt::CLI::detail::is_direct_constructible<T, C>::test [with T=std::vector<std::string, std::allocator<std::string>>, C=double]" based on template arguments <std::vector<std::string, std::allocator<std::string>>, double> at line 1041
            instantiation of class "vt::CLI::detail::is_direct_constructible<T, C> [with T=std::vector<std::string, std::allocator<std::string>>, C=double]" at line 5005
            instantiation of "void vt::CLI::Option::results(T &) const [with T=std::vector<std::string, std::allocator<std::string>>]" at line 5034
            instantiation of "T vt::CLI::Option::as<T>() const [with T=std::vector<std::string, std::allocator<std::string>>]" at line 7315

Remark: The warnings can be suppressed with "-diag-suppress <warning-number>"

/vt/lib/CLI/CLI/CLI11.hpp(1029): warning #2361-D: invalid narrowing conversion from "int" to "unsigned long"
          TT { std::declval<CC>() }
               ^
          detected during:
            instantiation of "vt::CLI::detail::is_direct_constructible<T, C>::test [with T=std::vector<std::string, std::allocator<std::string>>, C=int]" based on template arguments <std::vector<std::string, std::allocator<std::string>>, int> at line 1041
            instantiation of class "vt::CLI::detail::is_direct_constructible<T, C> [with T=std::vector<std::string, std::allocator<std::string>>, C=int]" at line 5005
            instantiation of "void vt::CLI::Option::results(T &) const [with T=std::vector<std::string, std::allocator<std::string>>]" at line 5034
            instantiation of "T vt::CLI::Option::as<T>() const [with T=std::vector<std::string, std::allocator<std::string>>]" at line 7315

Testing - passed

Build log

PR tests (gcc-12, ubuntu, mpich, verbose)

Build for 8eda5a8a7a52e6830b97b1f73db24969422a3ef2 (2024-05-30 19:00:18 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (intel icpx, ubuntu, mpich, verbose)

Build for 531c3a8e1218de56c4d5086dea793f818e763a42 (2024-07-17 20:10:33 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (clang-14, ubuntu, mpich, verbose)

Build for 531c3a8e1218de56c4d5086dea793f818e763a42 (2024-07-17 20:10:33 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (nvidia cuda 12.2.0, gcc-9, ubuntu, mpich, verbose)

Build for 531c3a8e1218de56c4d5086dea793f818e763a42 (2024-07-17 20:10:33 UTC)

/vt/lib/CLI/CLI/CLI11.hpp(1029): warning #2361-D: invalid narrowing conversion from "double" to "unsigned long"
          TT { std::declval<CC>() }
               ^
          detected during:
            instantiation of "vt::CLI::detail::is_direct_constructible<T, C>::test [with T=std::vector<std::string, std::allocator<std::string>>, C=double]" based on template arguments <std::vector<std::string, std::allocator<std::string>>, double> at line 1041
            instantiation of class "vt::CLI::detail::is_direct_constructible<T, C> [with T=std::vector<std::string, std::allocator<std::string>>, C=double]" at line 5005
            instantiation of "void vt::CLI::Option::results(T &) const [with T=std::vector<std::string, std::allocator<std::string>>]" at line 5034
            instantiation of "T vt::CLI::Option::as<T>() const [with T=std::vector<std::string, std::allocator<std::string>>]" at line 7315

Remark: The warnings can be suppressed with "-diag-suppress <warning-number>"

/vt/lib/CLI/CLI/CLI11.hpp(1029): warning #2361-D: invalid narrowing conversion from "int" to "unsigned long"
          TT { std::declval<CC>() }
               ^
          detected during:
            instantiation of "vt::CLI::detail::is_direct_constructible<T, C>::test [with T=std::vector<std::string, std::allocator<std::string>>, C=int]" based on template arguments <std::vector<std::string, std::allocator<std::string>>, int> at line 1041
            instantiation of class "vt::CLI::detail::is_direct_constructible<T, C> [with T=std::vector<std::string, std::allocator<std::string>>, C=int]" at line 5005
            instantiation of "void vt::CLI::Option::results(T &) const [with T=std::vector<std::string, std::allocator<std::string>>]" at line 5034
            instantiation of "T vt::CLI::Option::as<T>() const [with T=std::vector<std::string, std::allocator<std::string>>]" at line 7315

/vt/tests/perf/send_cost.cc(169): warning #177-D: variable "prevNode" was declared but never referenced
    auto const prevNode = (thisNode - 1 + num_nodes_) % num_nodes_;
               ^

Remark: The warnings can be suppressed with "-diag-suppress <warning-number>"

Testing - passed

Build log

PR tests (gcc-12, ubuntu, mpich, verbose, kokkos)

Build for 531c3a8e1218de56c4d5086dea793f818e763a42 (2024-07-17 20:10:33 UTC)

Compilation - successful

Testing - passed

Build log

cwschilly commented 9 months ago

I'm not sure what's going on with the PR checks (git --check) pipeline--there isn't any trailing white space in the file it's talking about

JacobDomagala commented 9 months ago

I think we should strip it down to bare minimum. We don't need test and contributing dirs. That would also resolve the trailing whitespace issue : )

Should we support using external yaml-cpp? Same way we intent to do with fmt (https://github.com/DARMA-tasking/vt/pull/2221)

cwschilly commented 9 months ago

Should we support using external yaml-cpp? Same way we intent to do with fmt (#2221)

I think this makes sense, especially since it seems the yaml-cpp source code doesn't pass the CI

JacobDomagala commented 9 months ago

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.

lifflander commented 9 months ago

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.

cwschilly commented 9 months ago

I've taken the approach of this PR (using enum classes) in order to resolve the clang warnings.

lifflander commented 5 months ago

@cwschilly Summary of our discussion:

Let's output the current configuration as a YAML file
Use this output for testing input is working properly
For the debug print configuration, take a list of components to output debug print statements
Create further nesting for inputs, like the "LB Data Output"/"Statistics"/"LB Data Input"

cz4rs commented 3 months ago

@cwschilly I have started to look at this, I will finish the review and post the comments tomorrow.

cz4rs commented 3 months ago

The order of precedence is currently
1. input yaml

2. command line arguments

3. input `toml` or `ini` file,
meaning that the YAML will overwrite the command line arguments (which in turn overwrite the input toml file).

Intuitively I'd expect the command line arguments to overwrite the configuration files. Whatever the order ends up being, this needs to be mentioned explicitly in the docs and possibly in usage / help message in the library.

lifflander commented 2 months ago

@cwschilly Can you edit the wiki to add yaml-cpp as one of our dependencies?

lifflander commented 2 months ago

Can you rebase this?

DARMA-tasking / vt