DPL: Allow passing labeled arrays through command line

Framework/Core/include/Framework/ConfigParamsHelper.h

@@ -37,6 +37,14 @@ struct ConfigParamsHelper {
                                options_description& options,
                                boost::program_options::options_description const& vetos = options_description());
 
+  /// Maximum length of a default value shown in the help, so that
+  /// an option and its description fit into a typical terminal line
+  static constexpr size_t MaxHelpDefaultLength = 40;
+
+  /// Text to show in the help for the default @a value: values longer than
+  /// MaxHelpDefaultLength are reduced to their beginning and end, separated by "..."
+  static std::string defaultValueForHelp(std::string const& value);
+
   /// Check if option is defined
   static bool hasOption(const std::vector<ConfigParamSpec>& specs, const std::string& optName);
 
@@ -112,7 +120,12 @@ struct ConfigParamsHelper {
       using Type = typename variant_type<V>::type;
       using BoostType = typename std::conditional<V == VariantType::String, std::string, Type>::type;
       auto value = boost::program_options::value<BoostType>();
-      value = value->default_value(spec.defaultValue.get<BoostType>());
+      if constexpr (V == VariantType::String) {
+        auto defaultValue = spec.defaultValue.get<BoostType>();
+        value = value->default_value(defaultValue, defaultValueForHelp(defaultValue));
+      } else {
+        value = value->default_value(spec.defaultValue.get<BoostType>());
+      }
       if constexpr (V == VariantType::Bool) {
         // for bool values we also support the zero_token option to make
         // the option usable as a single switch
@@ -121,11 +134,18 @@ struct ConfigParamsHelper {
       options.add_options()(name, value, help);
     } else if constexpr (isArray<V>() || isArray2D<V>()) {
       auto value = boost::program_options::value<std::string>();
-      value = value->default_value(spec.defaultValue.asString());
+      auto defaultValue = spec.defaultValue.asString();
+      value = value->default_value(defaultValue, defaultValueForHelp(defaultValue));
       if constexpr (V != VariantType::String) {
         value = value->multitoken();
       }
       options.add_options()(name, value, help);
+    } else if constexpr (isLabeledArray<V>()) {
+      // the JSON representation is passed as a single argument
+      auto value = boost::program_options::value<std::string>();
+      auto defaultValue = spec.defaultValue.asString();
+      value = value->default_value(defaultValue, defaultValueForHelp(defaultValue));
+      options.add_options()(name, value, help);
     } else {
       using Type = typename variant_type<V>::type;
       using BoostType = typename std::conditional<V == VariantType::String, std::string, Type>::type;

Framework/Core/include/Framework/Variant.h

@@ -403,6 +403,7 @@ class Variant
     *this = Variant(value, size);
   }
 
+  /// FIXME: set for vector of strings is not used anywhere, why?
   template <typename T>
   void set(std::vector<T>& values)
     requires(std::is_pod_v<T> || std::is_same_v<T, std::string>)

Framework/Core/include/Framework/VariantPropertyTreeHelpers.h

@@ -12,7 +12,6 @@
 #define FRAMEWORK_VARIANTPTREEHELPERS_H
 
 #include "Array2D.h"
-#include "Framework/Variant.h"
 #include <boost/property_tree/ptree.hpp>
 
 namespace o2::framework

Framework/Core/src/BoostOptionsRetriever.cxx

@@ -91,13 +91,13 @@ void BoostOptionsRetriever::update(std::vector<ConfigParamSpec> const& specs,
       case VariantType::Array2DInt:
       case VariantType::Array2DFloat:
       case VariantType::Array2DDouble:
-        options = options(name, bpo::value<std::string>()->default_value(spec.defaultValue.asString()), help);
-        break;
-      case VariantType::Dict:
       case VariantType::LabeledArrayInt:
       case VariantType::LabeledArrayFloat:
       case VariantType::LabeledArrayDouble:
       case VariantType::LabeledArrayString:
+        options = options(name, bpo::value<std::string>()->default_value(spec.defaultValue.asString()), help);
+        break;
+      case VariantType::Dict:
       case VariantType::Unknown:
       case VariantType::Empty:
         break;

Framework/Core/src/ConfigParamsHelper.cxx

@@ -13,6 +13,7 @@
 #include <boost/program_options.hpp>
 
 #include <string>
+#include <string_view>
 #include <vector>
 #include <iostream>
 
@@ -99,9 +100,17 @@ void ConfigParamsHelper::populateBoostProgramOptions(
         addConfigSpecOption<VariantType::Array2DDouble>(spec, options);
         break;
       case VariantType::LabeledArrayInt:
+        addConfigSpecOption<VariantType::LabeledArrayInt>(spec, options);
+        break;
       case VariantType::LabeledArrayFloat:
+        addConfigSpecOption<VariantType::LabeledArrayFloat>(spec, options);
+        break;
       case VariantType::LabeledArrayDouble:
+        addConfigSpecOption<VariantType::LabeledArrayDouble>(spec, options);
+        break;
       case VariantType::LabeledArrayString:
+        addConfigSpecOption<VariantType::LabeledArrayString>(spec, options);
+        break;
       // FIXME: for  Dict we should probably allow parsing stuff
       //        provided on the command line
       case VariantType::Dict:
@@ -112,6 +121,19 @@ void ConfigParamsHelper::populateBoostProgramOptions(
   }
 }
 
+std::string ConfigParamsHelper::defaultValueForHelp(std::string const& value)
+{
+  constexpr std::string_view ellipsis = "...";
+  if (value.size() <= MaxHelpDefaultLength) {
+    return value;
+  }
+  // Keep more of the beginning, which identifies the value, and the end, which closes it
+  constexpr size_t kept = MaxHelpDefaultLength - ellipsis.size();
+  constexpr size_t tail = kept / 3;
+  constexpr size_t head = kept - tail;
+  return value.substr(0, head) + std::string{ellipsis} + value.substr(value.size() - tail);
+}
+
 /// Check if option is defined
 bool ConfigParamsHelper::hasOption(const std::vector<ConfigParamSpec>& specs, const std::string& optName)
 {
@@ -158,7 +180,7 @@ bool ConfigParamsHelper::dpl2BoostOptions(const std::vector<ConfigParamSpec>& sp
     if (configSpec.type != VariantType::Bool) {
       if (configSpec.defaultValue.type() != VariantType::Empty) {
         options.add_options()(configSpec.name.c_str(),
-                              bpo::value<std::string>()->default_value(defaultValue.str()),
+                              bpo::value<std::string>()->default_value(defaultValue.str(), defaultValueForHelp(defaultValue.str())),
                               configSpec.help.c_str());
       } else {
         options.add_options()(configSpec.name.c_str(),

Framework/Core/src/PropertyTreeHelpers.cxx

@@ -35,6 +35,32 @@ inline Variant fromString(std::string const& str)
   ss.str(str);
   return VariantJSONHelpers::read<T>(ss);
 }
+
+template <typename T>
+auto replaceLabels(LabeledArray<T>& input, LabeledArray<T>&& spec)
+{
+  if (!input.getLabelsCols().empty() || !input.getLabelsRows().empty()) {
+    return false;
+  }
+  if (spec.getLabelsCols().empty() == false) {
+    input.replaceLabelsCols(spec.getLabelsCols());
+  }
+  if (spec.getLabelsRows().empty() == false) {
+    input.replaceLabelsRows(spec.getLabelsRows());
+  }
+  return true;
+}
+
+/// Special case for a labeled array to restore default labels that were omitted
+template <typename T, VariantType V>
+boost::property_tree::ptree labeledArrayFromStringToBranch(std::string const& str, Variant const& defaultValue)
+{
+  auto v = fromString<V>(str).template get<LabeledArray<T>>();
+  if (defaultValue.type() == V) {
+    replaceLabels(v, defaultValue.get<LabeledArray<T>>());
+  }
+  return labeledArrayToBranch(std::move(v));
+}
 } // namespace
 void PropertyTreeHelpers::populateDefaults(std::vector<ConfigParamSpec> const& schema,
                                            boost::property_tree::ptree& pt,
@@ -233,22 +259,18 @@ void PropertyTreeHelpers::populate(std::vector<ConfigParamSpec> const& schema,
           auto v = fromString<VariantType::Array2DDouble>(vmap[key].as<std::string>());
           pt.put_child(key, array2DToBranch<double>(v.get<Array2D<double>>()));
         } break;
-        case VariantType::LabeledArrayInt: {
-          auto v = fromString<VariantType::LabeledArrayInt>(vmap[key].as<std::string>());
-          pt.put_child(key, labeledArrayToBranch(v.get<LabeledArray<int>>()));
-        } break;
-        case VariantType::LabeledArrayFloat: {
-          auto v = fromString<VariantType::LabeledArrayFloat>(vmap[key].as<std::string>());
-          pt.put_child(key, labeledArrayToBranch(v.get<LabeledArray<float>>()));
-        } break;
-        case VariantType::LabeledArrayDouble: {
-          auto v = fromString<VariantType::LabeledArrayDouble>(vmap[key].as<std::string>());
-          pt.put_child(key, labeledArrayToBranch(v.get<LabeledArray<double>>()));
-        } break;
-        case VariantType::LabeledArrayString: {
-          auto v = fromString<VariantType::LabeledArrayString>(vmap[key].as<std::string>());
-          pt.put_child(key, labeledArrayToBranch(v.get<LabeledArray<std::string>>()));
-        } break;
+        case VariantType::LabeledArrayInt:
+          pt.put_child(key, labeledArrayFromStringToBranch<int, VariantType::LabeledArrayInt>(vmap[key].as<std::string>(), spec.defaultValue));
+          break;
+        case VariantType::LabeledArrayFloat:
+          pt.put_child(key, labeledArrayFromStringToBranch<float, VariantType::LabeledArrayFloat>(vmap[key].as<std::string>(), spec.defaultValue));
+          break;
+        case VariantType::LabeledArrayDouble:
+          pt.put_child(key, labeledArrayFromStringToBranch<double, VariantType::LabeledArrayDouble>(vmap[key].as<std::string>(), spec.defaultValue));
+          break;
+        case VariantType::LabeledArrayString:
+          pt.put_child(key, labeledArrayFromStringToBranch<std::string, VariantType::LabeledArrayString>(vmap[key].as<std::string>(), spec.defaultValue));
+          break;
         case VariantType::Dict:
           pt.put_child(key, vmap[key].as<boost::property_tree::ptree>());
           break;
@@ -272,21 +294,6 @@ void PropertyTreeHelpers::populate(std::vector<ConfigParamSpec> const& schema,
   O2_SIGNPOST_END(configuration, cid, "populate", "Done");
 }
 
-template <typename T>
-auto replaceLabels(LabeledArray<T>& input, LabeledArray<T>&& spec)
-{
-  if (!input.getLabelsCols().empty() || !input.getLabelsRows().empty()) {
-    return false;
-  }
-  if (spec.getLabelsCols().empty() == false) {
-    input.replaceLabelsCols(spec.getLabelsCols());
-  }
-  if (spec.getLabelsRows().empty() == false) {
-    input.replaceLabelsRows(spec.getLabelsRows());
-  }
-  return true;
-}
-
 void PropertyTreeHelpers::populate(std::vector<ConfigParamSpec> const& schema,
                                    boost::property_tree::ptree& pt,
                                    boost::property_tree::ptree const& in,

Framework/Core/src/Variant.cxx

@@ -66,6 +66,10 @@ std::ostream& operator<<(std::ostream& oss, Variant const& val)
     case VariantType::Array2DInt:
     case VariantType::Array2DFloat:
     case VariantType::Array2DDouble:
+    case VariantType::LabeledArrayInt:
+    case VariantType::LabeledArrayFloat:
+    case VariantType::LabeledArrayDouble:
+    case VariantType::LabeledArrayString:
       VariantJSONHelpers::write(oss, val);
       break;
     case VariantType::Empty:

Framework/Core/test/test_BoostOptionsRetriever.cxx

@@ -11,9 +11,14 @@
 
 #include "Framework/ConfigParamsHelper.h"
 #include "Framework/ConfigParamSpec.h"
+#include "Framework/BoostOptionsRetriever.h"
+#include "Framework/ConfigParamRegistry.h"
+#include "Framework/ConfigParamStore.h"
+#include "Framework/DataProcessorSpec.h"
 #include <boost/program_options.hpp>
 #include <boost/program_options/parsers.hpp>
 #include <catch_amalgamated.hpp>
+#include <sstream>
 #include <vector>
 
 using namespace o2::framework;
@@ -70,3 +75,133 @@ TEST_CASE("TrivialBoostOptionsRetrieverTest")
   REQUIRE(vm["someFloat"].as<float>() == 0.5);
   REQUIRE(vm["someDouble"].as<double>() == 0.5);
 }
+
+namespace
+{
+constexpr char const* fullLabeledJSON = R"({"labels_rows":["a","b"],"labels_cols":["x","y"],"values":[[5,6],[7,8]]})";
+
+LabeledArray<int> defaultLabeledArray()
+{
+  int m[2][2] = {{1, 2}, {3, 4}};
+  return LabeledArray<int>{&m[0][0], 2, 2, {"r1", "r2"}, {"c1", "c2"}};
+}
+
+std::unique_ptr<ConfigParamRegistry> registryFromCommandLine(std::vector<ConfigParamSpec> const& specs, std::vector<char const*> args)
+{
+  args.insert(args.begin(), "test");
+  std::vector<std::unique_ptr<ParamRetriever>> retrievers;
+  retrievers.emplace_back(std::make_unique<BoostOptionsRetriever>(true, args.size(), const_cast<char**>(args.data())));
+  auto store = std::make_unique<ConfigParamStore>(specs, std::move(retrievers));
+  store->preload();
+  store->activate();
+  return std::make_unique<ConfigParamRegistry>(std::move(store));
+}
+} // namespace
+
+TEST_CASE("LabeledArrayBoostOptionsTest")
+{
+  namespace bpo = boost::program_options;
+  auto specs = std::vector<ConfigParamSpec>{
+    {"someLabeledArray", VariantType::LabeledArrayInt, defaultLabeledArray(), {"some labeled array option"}}};
+
+  // the option is registered as a string holding the JSON representation
+  bpo::options_description opts;
+  ConfigParamsHelper::populateBoostProgramOptions(opts, specs);
+  const char* args[] = {"test", "--someLabeledArray", fullLabeledJSON};
+  bpo::variables_map vm;
+  bpo::store(parse_command_line(sizeof(args) / sizeof(char*), args, opts), vm);
+  bpo::notify(vm);
+  REQUIRE(vm["someLabeledArray"].as<std::string>() == fullLabeledJSON);
+
+  // the default is the JSON representation of the default value
+  bpo::variables_map vmDefault;
+  const char* noArgs[] = {"test"};
+  bpo::store(parse_command_line(1, noArgs, opts), vmDefault);
+  REQUIRE(vmDefault["someLabeledArray"].as<std::string>() == specs[0].defaultValue.asString());
+}
+
+TEST_CASE("LabeledArrayBoostOptionsRetrieverTest")
+{
+  auto specs = std::vector<ConfigParamSpec>{
+    {"someLabeledArray", VariantType::LabeledArrayInt, defaultLabeledArray(), {"some labeled array option"}}};
+
+  // full JSON on the command line, its labels are kept
+  auto registry = registryFromCommandLine(specs, {"--someLabeledArray", fullLabeledJSON});
+  auto la = registry->get<LabeledArray<int>>("someLabeledArray");
+  REQUIRE(la.getLabelsRows() == std::vector<std::string>{"a", "b"});
+  REQUIRE(la.getLabelsCols() == std::vector<std::string>{"x", "y"});
+  REQUIRE(la.get("b", "y") == 8);
+
+  // absent from the command line: the default value
+  registry = registryFromCommandLine(specs, {});
+  la = registry->get<LabeledArray<int>>("someLabeledArray");
+  REQUIRE(la.get("r2", "c2") == 4);
+
+  // only values on the command line: the default labels are used
+  registry = registryFromCommandLine(specs, {"--someLabeledArray", R"({"values":[[5,6],[7,8]]})"});
+  la = registry->get<LabeledArray<int>>("someLabeledArray");
+  REQUIRE(la.getLabelsRows() == std::vector<std::string>{"r1", "r2"});
+  REQUIRE(la.getLabelsCols() == std::vector<std::string>{"c1", "c2"});
+  REQUIRE(la.get("r2", "c1") == 7);
+
+  // values which do not match the default labels
+  REQUIRE_THROWS(registryFromCommandLine(specs, {"--someLabeledArray", R"({"values":[[5,6,7]]})"}));
+}
+
+TEST_CASE("DefaultValueForHelpTest")
+{
+  auto const max = ConfigParamsHelper::MaxHelpDefaultLength;
+  REQUIRE(ConfigParamsHelper::defaultValueForHelp("") == "");
+  REQUIRE(ConfigParamsHelper::defaultValueForHelp("short") == "short");
+  std::string atLimit(max, 'x');
+  REQUIRE(ConfigParamsHelper::defaultValueForHelp(atLimit) == atLimit);
+
+  std::string longValue = std::string("BEGIN") + std::string(100, '-') + "END";
+  auto shown = ConfigParamsHelper::defaultValueForHelp(longValue);
+  REQUIRE(shown.size() == max);
+  REQUIRE(shown.starts_with("BEGIN"));
+  REQUIRE(shown.ends_with("END"));
+  REQUIRE(shown.find("...") != std::string::npos);
+  REQUIRE(ConfigParamsHelper::defaultValueForHelp(std::string(max + 1, 'x')).size() == max);
+}
+
+TEST_CASE("LongDefaultInHelpTest")
+{
+  namespace bpo = boost::program_options;
+  std::vector<int> values(30, 12345);
+  std::vector<std::string> rows{"row"};
+  std::vector<std::string> cols;
+  for (auto i = 0U; i < values.size(); ++i) {
+    cols.push_back("column" + std::to_string(i));
+  }
+  LabeledArray<int> la{values.data(), 1, static_cast<uint32_t>(values.size()), rows, cols};
+  auto workflowOptions = std::vector<ConfigParamSpec>{
+    {"longLabeledArray", VariantType::LabeledArrayInt, la, {"a labeled array option with a long default"}}};
+  auto full = workflowOptions[0].defaultValue.asString();
+  REQUIRE(full.size() > ConfigParamsHelper::MaxHelpDefaultLength);
+
+  // the help shows the abbreviated default only
+  auto description = ConfigParamsHelper::prepareOptionDescriptions(std::vector<DataProcessorSpec>{}, workflowOptions);
+  std::stringstream help;
+  help << description;
+  REQUIRE(help.str().find(ConfigParamsHelper::defaultValueForHelp(full)) != std::string::npos);
+  REQUIRE(help.str().find(full) == std::string::npos);
+
+  // the default value used when parsing is still the full one
+  bpo::options_description forwarded;
+  ConfigParamsHelper::dpl2BoostOptions(workflowOptions, forwarded);
+  const char* noArgs[] = {"test"};
+  bpo::variables_map vm;
+  bpo::store(parse_command_line(1, noArgs, forwarded), vm);
+  REQUIRE(vm["longLabeledArray"].as<std::string>() == full);
+
+  // also for the options registered for the devices
+  bpo::options_description deviceOptions;
+  ConfigParamsHelper::populateBoostProgramOptions(deviceOptions, workflowOptions);
+  std::stringstream deviceHelp;
+  deviceHelp << deviceOptions;
+  REQUIRE(deviceHelp.str().find(full) == std::string::npos);
+  bpo::variables_map deviceVm;
+  bpo::store(parse_command_line(1, noArgs, deviceOptions), deviceVm);
+  REQUIRE(deviceVm["longLabeledArray"].as<std::string>() == full);
+}