Skip to content

Commit a544caf

Browse files
committed
Allow passing LabeledArrays as commandline options
* Protect from long default values in `--help` output by adding a size limit and manually adding reduced string representation * Use pre-existing to/from JSON conversions to dump and retrieve labeled arrays in command line * Allow passing only values if the labels do not change from defaults * Test options retrieval for the driver and devices
1 parent eff3ac1 commit a544caf

11 files changed

Lines changed: 349 additions & 47 deletions

‎Framework/Core/include/Framework/ConfigParamsHelper.h‎

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,14 @@ struct ConfigParamsHelper {
3737
options_description& options,
3838
boost::program_options::options_description const& vetos = options_description());
3939

40+
/// Maximum length of a default value shown in the help, so that
41+
/// an option and its description fit into a typical terminal line
42+
static constexpr size_t MaxHelpDefaultLength = 40;
43+
44+
/// Text to show in the help for the default @a value: values longer than
45+
/// MaxHelpDefaultLength are reduced to their beginning and end, separated by "..."
46+
static std::string defaultValueForHelp(std::string const& value);
47+
4048
/// Check if option is defined
4149
static bool hasOption(const std::vector<ConfigParamSpec>& specs, const std::string& optName);
4250

@@ -112,7 +120,12 @@ struct ConfigParamsHelper {
112120
using Type = typename variant_type<V>::type;
113121
using BoostType = typename std::conditional<V == VariantType::String, std::string, Type>::type;
114122
auto value = boost::program_options::value<BoostType>();
115-
value = value->default_value(spec.defaultValue.get<BoostType>());
123+
if constexpr (V == VariantType::String) {
124+
auto defaultValue = spec.defaultValue.get<BoostType>();
125+
value = value->default_value(defaultValue, defaultValueForHelp(defaultValue));
126+
} else {
127+
value = value->default_value(spec.defaultValue.get<BoostType>());
128+
}
116129
if constexpr (V == VariantType::Bool) {
117130
// for bool values we also support the zero_token option to make
118131
// the option usable as a single switch
@@ -121,11 +134,18 @@ struct ConfigParamsHelper {
121134
options.add_options()(name, value, help);
122135
} else if constexpr (isArray<V>() || isArray2D<V>()) {
123136
auto value = boost::program_options::value<std::string>();
124-
value = value->default_value(spec.defaultValue.asString());
137+
auto defaultValue = spec.defaultValue.asString();
138+
value = value->default_value(defaultValue, defaultValueForHelp(defaultValue));
125139
if constexpr (V != VariantType::String) {
126140
value = value->multitoken();
127141
}
128142
options.add_options()(name, value, help);
143+
} else if constexpr (isLabeledArray<V>()) {
144+
// the JSON representation is passed as a single argument
145+
auto value = boost::program_options::value<std::string>();
146+
auto defaultValue = spec.defaultValue.asString();
147+
value = value->default_value(defaultValue, defaultValueForHelp(defaultValue));
148+
options.add_options()(name, value, help);
129149
} else {
130150
using Type = typename variant_type<V>::type;
131151
using BoostType = typename std::conditional<V == VariantType::String, std::string, Type>::type;

‎Framework/Core/include/Framework/VariantPropertyTreeHelpers.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
#define FRAMEWORK_VARIANTPTREEHELPERS_H
1313

1414
#include "Array2D.h"
15-
#include "Framework/Variant.h"
1615
#include <boost/property_tree/ptree.hpp>
1716

1817
namespace o2::framework

‎Framework/Core/src/BoostOptionsRetriever.cxx‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,13 +91,13 @@ void BoostOptionsRetriever::update(std::vector<ConfigParamSpec> const& specs,
9191
case VariantType::Array2DInt:
9292
case VariantType::Array2DFloat:
9393
case VariantType::Array2DDouble:
94-
options = options(name, bpo::value<std::string>()->default_value(spec.defaultValue.asString()), help);
95-
break;
96-
case VariantType::Dict:
9794
case VariantType::LabeledArrayInt:
9895
case VariantType::LabeledArrayFloat:
9996
case VariantType::LabeledArrayDouble:
10097
case VariantType::LabeledArrayString:
98+
options = options(name, bpo::value<std::string>()->default_value(spec.defaultValue.asString()), help);
99+
break;
100+
case VariantType::Dict:
101101
case VariantType::Unknown:
102102
case VariantType::Empty:
103103
break;

‎Framework/Core/src/ConfigParamsHelper.cxx‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include <boost/program_options.hpp>
1414

1515
#include <string>
16+
#include <string_view>
1617
#include <vector>
1718
#include <iostream>
1819

@@ -99,9 +100,17 @@ void ConfigParamsHelper::populateBoostProgramOptions(
99100
addConfigSpecOption<VariantType::Array2DDouble>(spec, options);
100101
break;
101102
case VariantType::LabeledArrayInt:
103+
addConfigSpecOption<VariantType::LabeledArrayInt>(spec, options);
104+
break;
102105
case VariantType::LabeledArrayFloat:
106+
addConfigSpecOption<VariantType::LabeledArrayFloat>(spec, options);
107+
break;
103108
case VariantType::LabeledArrayDouble:
109+
addConfigSpecOption<VariantType::LabeledArrayDouble>(spec, options);
110+
break;
104111
case VariantType::LabeledArrayString:
112+
addConfigSpecOption<VariantType::LabeledArrayString>(spec, options);
113+
break;
105114
// FIXME: for Dict we should probably allow parsing stuff
106115
// provided on the command line
107116
case VariantType::Dict:
@@ -112,6 +121,19 @@ void ConfigParamsHelper::populateBoostProgramOptions(
112121
}
113122
}
114123

124+
std::string ConfigParamsHelper::defaultValueForHelp(std::string const& value)
125+
{
126+
constexpr std::string_view ellipsis = "...";
127+
if (value.size() <= MaxHelpDefaultLength) {
128+
return value;
129+
}
130+
// Keep more of the beginning, which identifies the value, and the end, which closes it
131+
constexpr size_t kept = MaxHelpDefaultLength - ellipsis.size();
132+
constexpr size_t tail = kept / 3;
133+
constexpr size_t head = kept - tail;
134+
return value.substr(0, head) + std::string{ellipsis} + value.substr(value.size() - tail);
135+
}
136+
115137
/// Check if option is defined
116138
bool ConfigParamsHelper::hasOption(const std::vector<ConfigParamSpec>& specs, const std::string& optName)
117139
{
@@ -158,7 +180,7 @@ bool ConfigParamsHelper::dpl2BoostOptions(const std::vector<ConfigParamSpec>& sp
158180
if (configSpec.type != VariantType::Bool) {
159181
if (configSpec.defaultValue.type() != VariantType::Empty) {
160182
options.add_options()(configSpec.name.c_str(),
161-
bpo::value<std::string>()->default_value(defaultValue.str()),
183+
bpo::value<std::string>()->default_value(defaultValue.str(), defaultValueForHelp(defaultValue.str())),
162184
configSpec.help.c_str());
163185
} else {
164186
options.add_options()(configSpec.name.c_str(),

‎Framework/Core/src/PropertyTreeHelpers.cxx‎

Lines changed: 38 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,32 @@ inline Variant fromString(std::string const& str)
3535
ss.str(str);
3636
return VariantJSONHelpers::read<T>(ss);
3737
}
38+
39+
template <typename T>
40+
auto replaceLabels(LabeledArray<T>& input, LabeledArray<T>&& spec)
41+
{
42+
if (!input.getLabelsCols().empty() || !input.getLabelsRows().empty()) {
43+
return false;
44+
}
45+
if (spec.getLabelsCols().empty() == false) {
46+
input.replaceLabelsCols(spec.getLabelsCols());
47+
}
48+
if (spec.getLabelsRows().empty() == false) {
49+
input.replaceLabelsRows(spec.getLabelsRows());
50+
}
51+
return true;
52+
}
53+
54+
/// Special case for a labeled array to restore default labels that were omitted
55+
template <typename T, VariantType V>
56+
boost::property_tree::ptree labeledArrayFromStringToBranch(std::string const& str, Variant const& defaultValue)
57+
{
58+
auto v = fromString<V>(str).template get<LabeledArray<T>>();
59+
if (defaultValue.type() == V) {
60+
replaceLabels(v, defaultValue.get<LabeledArray<T>>());
61+
}
62+
return labeledArrayToBranch(std::move(v));
63+
}
3864
} // namespace
3965
void PropertyTreeHelpers::populateDefaults(std::vector<ConfigParamSpec> const& schema,
4066
boost::property_tree::ptree& pt,
@@ -233,22 +259,18 @@ void PropertyTreeHelpers::populate(std::vector<ConfigParamSpec> const& schema,
233259
auto v = fromString<VariantType::Array2DDouble>(vmap[key].as<std::string>());
234260
pt.put_child(key, array2DToBranch<double>(v.get<Array2D<double>>()));
235261
} break;
236-
case VariantType::LabeledArrayInt: {
237-
auto v = fromString<VariantType::LabeledArrayInt>(vmap[key].as<std::string>());
238-
pt.put_child(key, labeledArrayToBranch(v.get<LabeledArray<int>>()));
239-
} break;
240-
case VariantType::LabeledArrayFloat: {
241-
auto v = fromString<VariantType::LabeledArrayFloat>(vmap[key].as<std::string>());
242-
pt.put_child(key, labeledArrayToBranch(v.get<LabeledArray<float>>()));
243-
} break;
244-
case VariantType::LabeledArrayDouble: {
245-
auto v = fromString<VariantType::LabeledArrayDouble>(vmap[key].as<std::string>());
246-
pt.put_child(key, labeledArrayToBranch(v.get<LabeledArray<double>>()));
247-
} break;
248-
case VariantType::LabeledArrayString: {
249-
auto v = fromString<VariantType::LabeledArrayString>(vmap[key].as<std::string>());
250-
pt.put_child(key, labeledArrayToBranch(v.get<LabeledArray<std::string>>()));
251-
} break;
262+
case VariantType::LabeledArrayInt:
263+
pt.put_child(key, labeledArrayFromStringToBranch<int, VariantType::LabeledArrayInt>(vmap[key].as<std::string>(), spec.defaultValue));
264+
break;
265+
case VariantType::LabeledArrayFloat:
266+
pt.put_child(key, labeledArrayFromStringToBranch<float, VariantType::LabeledArrayFloat>(vmap[key].as<std::string>(), spec.defaultValue));
267+
break;
268+
case VariantType::LabeledArrayDouble:
269+
pt.put_child(key, labeledArrayFromStringToBranch<double, VariantType::LabeledArrayDouble>(vmap[key].as<std::string>(), spec.defaultValue));
270+
break;
271+
case VariantType::LabeledArrayString:
272+
pt.put_child(key, labeledArrayFromStringToBranch<std::string, VariantType::LabeledArrayString>(vmap[key].as<std::string>(), spec.defaultValue));
273+
break;
252274
case VariantType::Dict:
253275
pt.put_child(key, vmap[key].as<boost::property_tree::ptree>());
254276
break;
@@ -272,21 +294,6 @@ void PropertyTreeHelpers::populate(std::vector<ConfigParamSpec> const& schema,
272294
O2_SIGNPOST_END(configuration, cid, "populate", "Done");
273295
}
274296

275-
template <typename T>
276-
auto replaceLabels(LabeledArray<T>& input, LabeledArray<T>&& spec)
277-
{
278-
if (!input.getLabelsCols().empty() || !input.getLabelsRows().empty()) {
279-
return false;
280-
}
281-
if (spec.getLabelsCols().empty() == false) {
282-
input.replaceLabelsCols(spec.getLabelsCols());
283-
}
284-
if (spec.getLabelsRows().empty() == false) {
285-
input.replaceLabelsRows(spec.getLabelsRows());
286-
}
287-
return true;
288-
}
289-
290297
void PropertyTreeHelpers::populate(std::vector<ConfigParamSpec> const& schema,
291298
boost::property_tree::ptree& pt,
292299
boost::property_tree::ptree const& in,

‎Framework/Core/src/Variant.cxx‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,10 @@ std::ostream& operator<<(std::ostream& oss, Variant const& val)
6666
case VariantType::Array2DInt:
6767
case VariantType::Array2DFloat:
6868
case VariantType::Array2DDouble:
69+
case VariantType::LabeledArrayInt:
70+
case VariantType::LabeledArrayFloat:
71+
case VariantType::LabeledArrayDouble:
72+
case VariantType::LabeledArrayString:
6973
VariantJSONHelpers::write(oss, val);
7074
break;
7175
case VariantType::Empty:

‎Framework/Core/test/test_BoostOptionsRetriever.cxx‎

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,14 @@
1111

1212
#include "Framework/ConfigParamsHelper.h"
1313
#include "Framework/ConfigParamSpec.h"
14+
#include "Framework/BoostOptionsRetriever.h"
15+
#include "Framework/ConfigParamRegistry.h"
16+
#include "Framework/ConfigParamStore.h"
17+
#include "Framework/DataProcessorSpec.h"
1418
#include <boost/program_options.hpp>
1519
#include <boost/program_options/parsers.hpp>
1620
#include <catch_amalgamated.hpp>
21+
#include <sstream>
1722
#include <vector>
1823

1924
using namespace o2::framework;
@@ -70,3 +75,133 @@ TEST_CASE("TrivialBoostOptionsRetrieverTest")
7075
REQUIRE(vm["someFloat"].as<float>() == 0.5);
7176
REQUIRE(vm["someDouble"].as<double>() == 0.5);
7277
}
78+
79+
namespace
80+
{
81+
constexpr char const* fullLabeledJSON = R"({"labels_rows":["a","b"],"labels_cols":["x","y"],"values":[[5,6],[7,8]]})";
82+
83+
LabeledArray<int> defaultLabeledArray()
84+
{
85+
int m[2][2] = {{1, 2}, {3, 4}};
86+
return LabeledArray<int>{&m[0][0], 2, 2, {"r1", "r2"}, {"c1", "c2"}};
87+
}
88+
89+
std::unique_ptr<ConfigParamRegistry> registryFromCommandLine(std::vector<ConfigParamSpec> const& specs, std::vector<char const*> args)
90+
{
91+
args.insert(args.begin(), "test");
92+
std::vector<std::unique_ptr<ParamRetriever>> retrievers;
93+
retrievers.emplace_back(std::make_unique<BoostOptionsRetriever>(true, args.size(), const_cast<char**>(args.data())));
94+
auto store = std::make_unique<ConfigParamStore>(specs, std::move(retrievers));
95+
store->preload();
96+
store->activate();
97+
return std::make_unique<ConfigParamRegistry>(std::move(store));
98+
}
99+
} // namespace
100+
101+
TEST_CASE("LabeledArrayBoostOptionsTest")
102+
{
103+
namespace bpo = boost::program_options;
104+
auto specs = std::vector<ConfigParamSpec>{
105+
{"someLabeledArray", VariantType::LabeledArrayInt, defaultLabeledArray(), {"some labeled array option"}}};
106+
107+
// the option is registered as a string holding the JSON representation
108+
bpo::options_description opts;
109+
ConfigParamsHelper::populateBoostProgramOptions(opts, specs);
110+
const char* args[] = {"test", "--someLabeledArray", fullLabeledJSON};
111+
bpo::variables_map vm;
112+
bpo::store(parse_command_line(sizeof(args) / sizeof(char*), args, opts), vm);
113+
bpo::notify(vm);
114+
REQUIRE(vm["someLabeledArray"].as<std::string>() == fullLabeledJSON);
115+
116+
// the default is the JSON representation of the default value
117+
bpo::variables_map vmDefault;
118+
const char* noArgs[] = {"test"};
119+
bpo::store(parse_command_line(1, noArgs, opts), vmDefault);
120+
REQUIRE(vmDefault["someLabeledArray"].as<std::string>() == specs[0].defaultValue.asString());
121+
}
122+
123+
TEST_CASE("LabeledArrayBoostOptionsRetrieverTest")
124+
{
125+
auto specs = std::vector<ConfigParamSpec>{
126+
{"someLabeledArray", VariantType::LabeledArrayInt, defaultLabeledArray(), {"some labeled array option"}}};
127+
128+
// full JSON on the command line, its labels are kept
129+
auto registry = registryFromCommandLine(specs, {"--someLabeledArray", fullLabeledJSON});
130+
auto la = registry->get<LabeledArray<int>>("someLabeledArray");
131+
REQUIRE(la.getLabelsRows() == std::vector<std::string>{"a", "b"});
132+
REQUIRE(la.getLabelsCols() == std::vector<std::string>{"x", "y"});
133+
REQUIRE(la.get("b", "y") == 8);
134+
135+
// absent from the command line: the default value
136+
registry = registryFromCommandLine(specs, {});
137+
la = registry->get<LabeledArray<int>>("someLabeledArray");
138+
REQUIRE(la.get("r2", "c2") == 4);
139+
140+
// only values on the command line: the default labels are used
141+
registry = registryFromCommandLine(specs, {"--someLabeledArray", R"({"values":[[5,6],[7,8]]})"});
142+
la = registry->get<LabeledArray<int>>("someLabeledArray");
143+
REQUIRE(la.getLabelsRows() == std::vector<std::string>{"r1", "r2"});
144+
REQUIRE(la.getLabelsCols() == std::vector<std::string>{"c1", "c2"});
145+
REQUIRE(la.get("r2", "c1") == 7);
146+
147+
// values which do not match the default labels
148+
REQUIRE_THROWS(registryFromCommandLine(specs, {"--someLabeledArray", R"({"values":[[5,6,7]]})"}));
149+
}
150+
151+
TEST_CASE("DefaultValueForHelpTest")
152+
{
153+
auto const max = ConfigParamsHelper::MaxHelpDefaultLength;
154+
REQUIRE(ConfigParamsHelper::defaultValueForHelp("") == "");
155+
REQUIRE(ConfigParamsHelper::defaultValueForHelp("short") == "short");
156+
std::string atLimit(max, 'x');
157+
REQUIRE(ConfigParamsHelper::defaultValueForHelp(atLimit) == atLimit);
158+
159+
std::string longValue = std::string("BEGIN") + std::string(100, '-') + "END";
160+
auto shown = ConfigParamsHelper::defaultValueForHelp(longValue);
161+
REQUIRE(shown.size() == max);
162+
REQUIRE(shown.starts_with("BEGIN"));
163+
REQUIRE(shown.ends_with("END"));
164+
REQUIRE(shown.find("...") != std::string::npos);
165+
REQUIRE(ConfigParamsHelper::defaultValueForHelp(std::string(max + 1, 'x')).size() == max);
166+
}
167+
168+
TEST_CASE("LongDefaultInHelpTest")
169+
{
170+
namespace bpo = boost::program_options;
171+
std::vector<int> values(30, 12345);
172+
std::vector<std::string> rows{"row"};
173+
std::vector<std::string> cols;
174+
for (auto i = 0U; i < values.size(); ++i) {
175+
cols.push_back("column" + std::to_string(i));
176+
}
177+
LabeledArray<int> la{values.data(), 1, static_cast<uint32_t>(values.size()), rows, cols};
178+
auto workflowOptions = std::vector<ConfigParamSpec>{
179+
{"longLabeledArray", VariantType::LabeledArrayInt, la, {"a labeled array option with a long default"}}};
180+
auto full = workflowOptions[0].defaultValue.asString();
181+
REQUIRE(full.size() > ConfigParamsHelper::MaxHelpDefaultLength);
182+
183+
// the help shows the abbreviated default only
184+
auto description = ConfigParamsHelper::prepareOptionDescriptions(std::vector<DataProcessorSpec>{}, workflowOptions);
185+
std::stringstream help;
186+
help << description;
187+
REQUIRE(help.str().find(ConfigParamsHelper::defaultValueForHelp(full)) != std::string::npos);
188+
REQUIRE(help.str().find(full) == std::string::npos);
189+
190+
// the default value used when parsing is still the full one
191+
bpo::options_description forwarded;
192+
ConfigParamsHelper::dpl2BoostOptions(workflowOptions, forwarded);
193+
const char* noArgs[] = {"test"};
194+
bpo::variables_map vm;
195+
bpo::store(parse_command_line(1, noArgs, forwarded), vm);
196+
REQUIRE(vm["longLabeledArray"].as<std::string>() == full);
197+
198+
// also for the options registered for the devices
199+
bpo::options_description deviceOptions;
200+
ConfigParamsHelper::populateBoostProgramOptions(deviceOptions, workflowOptions);
201+
std::stringstream deviceHelp;
202+
deviceHelp << deviceOptions;
203+
REQUIRE(deviceHelp.str().find(full) == std::string::npos);
204+
bpo::variables_map deviceVm;
205+
bpo::store(parse_command_line(1, noArgs, deviceOptions), deviceVm);
206+
REQUIRE(deviceVm["longLabeledArray"].as<std::string>() == full);
207+
}

0 commit comments

Comments
 (0)