From 8437380ba2024b99cf57dcb375baa11a55036c39 Mon Sep 17 00:00:00 2001 From: Anton Alkin Date: Thu, 1 Oct 2026 10:44:39 +0200 Subject: [PATCH 1/5] Fix ownership of the content of Variant Copy and move of Variants holding Array2D, LabeledArray or string array values copied the storage byte by byte, so copies shared the same heap buffers, and the destructor never destroyed these objects, leaking them. Assignments never released the previous content, move assignment of strings and arrays duplicated the buffer and dropped the original one, and move assignment of string arrays corrupted the source vector. --- Framework/Core/include/Framework/Variant.h | 1 + 1 file changed, 1 insertion(+) diff --git a/Framework/Core/include/Framework/Variant.h b/Framework/Core/include/Framework/Variant.h index c56c0c01ef9fc..e0bbccf560fad 100644 --- a/Framework/Core/include/Framework/Variant.h +++ b/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 void set(std::vector& values) requires(std::is_pod_v || std::is_same_v) From d5f1a773a8112a7b8e57c50052c76a5eb7029cc4 Mon Sep 17 00:00:00 2001 From: Anton Alkin Date: Thu, 1 Oct 2026 12:35:52 +0200 Subject: [PATCH 2/5] 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 --- .../include/Framework/ConfigParamsHelper.h | 24 +++- .../Framework/VariantPropertyTreeHelpers.h | 1 - Framework/Core/src/BoostOptionsRetriever.cxx | 6 +- Framework/Core/src/ConfigParamsHelper.cxx | 24 +++- Framework/Core/src/PropertyTreeHelpers.cxx | 69 +++++---- Framework/Core/src/Variant.cxx | 4 + .../Core/test/test_BoostOptionsRetriever.cxx | 135 ++++++++++++++++++ .../Core/test/test_DeviceSpecHelpers.cxx | 18 ++- .../Core/test/test_FairMQOptionsRetriever.cxx | 43 ++++++ Framework/Core/test/test_Variants.cxx | 44 ++++++ .../Core/test/test_WorkflowSerialization.cxx | 28 +++- 11 files changed, 349 insertions(+), 47 deletions(-) diff --git a/Framework/Core/include/Framework/ConfigParamsHelper.h b/Framework/Core/include/Framework/ConfigParamsHelper.h index 3783cdb374bd1..ceff4f47c4ec1 100644 --- a/Framework/Core/include/Framework/ConfigParamsHelper.h +++ b/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& specs, const std::string& optName); @@ -112,7 +120,12 @@ struct ConfigParamsHelper { using Type = typename variant_type::type; using BoostType = typename std::conditional::type; auto value = boost::program_options::value(); - value = value->default_value(spec.defaultValue.get()); + if constexpr (V == VariantType::String) { + auto defaultValue = spec.defaultValue.get(); + value = value->default_value(defaultValue, defaultValueForHelp(defaultValue)); + } else { + value = value->default_value(spec.defaultValue.get()); + } 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() || isArray2D()) { auto value = boost::program_options::value(); - 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()) { + // the JSON representation is passed as a single argument + auto value = boost::program_options::value(); + auto defaultValue = spec.defaultValue.asString(); + value = value->default_value(defaultValue, defaultValueForHelp(defaultValue)); + options.add_options()(name, value, help); } else { using Type = typename variant_type::type; using BoostType = typename std::conditional::type; diff --git a/Framework/Core/include/Framework/VariantPropertyTreeHelpers.h b/Framework/Core/include/Framework/VariantPropertyTreeHelpers.h index 05ab71d39b0e4..afc4a7f989802 100644 --- a/Framework/Core/include/Framework/VariantPropertyTreeHelpers.h +++ b/Framework/Core/include/Framework/VariantPropertyTreeHelpers.h @@ -12,7 +12,6 @@ #define FRAMEWORK_VARIANTPTREEHELPERS_H #include "Array2D.h" -#include "Framework/Variant.h" #include namespace o2::framework diff --git a/Framework/Core/src/BoostOptionsRetriever.cxx b/Framework/Core/src/BoostOptionsRetriever.cxx index fbf56a312cf66..ee559801fdb6e 100644 --- a/Framework/Core/src/BoostOptionsRetriever.cxx +++ b/Framework/Core/src/BoostOptionsRetriever.cxx @@ -91,13 +91,13 @@ void BoostOptionsRetriever::update(std::vector const& specs, case VariantType::Array2DInt: case VariantType::Array2DFloat: case VariantType::Array2DDouble: - options = options(name, bpo::value()->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()->default_value(spec.defaultValue.asString()), help); + break; + case VariantType::Dict: case VariantType::Unknown: case VariantType::Empty: break; diff --git a/Framework/Core/src/ConfigParamsHelper.cxx b/Framework/Core/src/ConfigParamsHelper.cxx index fc59710fd71a0..77532a3796468 100644 --- a/Framework/Core/src/ConfigParamsHelper.cxx +++ b/Framework/Core/src/ConfigParamsHelper.cxx @@ -13,6 +13,7 @@ #include #include +#include #include #include @@ -99,9 +100,17 @@ void ConfigParamsHelper::populateBoostProgramOptions( addConfigSpecOption(spec, options); break; case VariantType::LabeledArrayInt: + addConfigSpecOption(spec, options); + break; case VariantType::LabeledArrayFloat: + addConfigSpecOption(spec, options); + break; case VariantType::LabeledArrayDouble: + addConfigSpecOption(spec, options); + break; case VariantType::LabeledArrayString: + addConfigSpecOption(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& specs, const std::string& optName) { @@ -158,7 +180,7 @@ bool ConfigParamsHelper::dpl2BoostOptions(const std::vector& sp if (configSpec.type != VariantType::Bool) { if (configSpec.defaultValue.type() != VariantType::Empty) { options.add_options()(configSpec.name.c_str(), - bpo::value()->default_value(defaultValue.str()), + bpo::value()->default_value(defaultValue.str(), defaultValueForHelp(defaultValue.str())), configSpec.help.c_str()); } else { options.add_options()(configSpec.name.c_str(), diff --git a/Framework/Core/src/PropertyTreeHelpers.cxx b/Framework/Core/src/PropertyTreeHelpers.cxx index 18055edb635b4..dc7e6c2130e96 100644 --- a/Framework/Core/src/PropertyTreeHelpers.cxx +++ b/Framework/Core/src/PropertyTreeHelpers.cxx @@ -35,6 +35,32 @@ inline Variant fromString(std::string const& str) ss.str(str); return VariantJSONHelpers::read(ss); } + +template +auto replaceLabels(LabeledArray& input, LabeledArray&& 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 +boost::property_tree::ptree labeledArrayFromStringToBranch(std::string const& str, Variant const& defaultValue) +{ + auto v = fromString(str).template get>(); + if (defaultValue.type() == V) { + replaceLabels(v, defaultValue.get>()); + } + return labeledArrayToBranch(std::move(v)); +} } // namespace void PropertyTreeHelpers::populateDefaults(std::vector const& schema, boost::property_tree::ptree& pt, @@ -233,22 +259,18 @@ void PropertyTreeHelpers::populate(std::vector const& schema, auto v = fromString(vmap[key].as()); pt.put_child(key, array2DToBranch(v.get>())); } break; - case VariantType::LabeledArrayInt: { - auto v = fromString(vmap[key].as()); - pt.put_child(key, labeledArrayToBranch(v.get>())); - } break; - case VariantType::LabeledArrayFloat: { - auto v = fromString(vmap[key].as()); - pt.put_child(key, labeledArrayToBranch(v.get>())); - } break; - case VariantType::LabeledArrayDouble: { - auto v = fromString(vmap[key].as()); - pt.put_child(key, labeledArrayToBranch(v.get>())); - } break; - case VariantType::LabeledArrayString: { - auto v = fromString(vmap[key].as()); - pt.put_child(key, labeledArrayToBranch(v.get>())); - } break; + case VariantType::LabeledArrayInt: + pt.put_child(key, labeledArrayFromStringToBranch(vmap[key].as(), spec.defaultValue)); + break; + case VariantType::LabeledArrayFloat: + pt.put_child(key, labeledArrayFromStringToBranch(vmap[key].as(), spec.defaultValue)); + break; + case VariantType::LabeledArrayDouble: + pt.put_child(key, labeledArrayFromStringToBranch(vmap[key].as(), spec.defaultValue)); + break; + case VariantType::LabeledArrayString: + pt.put_child(key, labeledArrayFromStringToBranch(vmap[key].as(), spec.defaultValue)); + break; case VariantType::Dict: pt.put_child(key, vmap[key].as()); break; @@ -272,21 +294,6 @@ void PropertyTreeHelpers::populate(std::vector const& schema, O2_SIGNPOST_END(configuration, cid, "populate", "Done"); } -template -auto replaceLabels(LabeledArray& input, LabeledArray&& 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 const& schema, boost::property_tree::ptree& pt, boost::property_tree::ptree const& in, diff --git a/Framework/Core/src/Variant.cxx b/Framework/Core/src/Variant.cxx index 35d20f85ba919..ad89e22a3bf39 100644 --- a/Framework/Core/src/Variant.cxx +++ b/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: diff --git a/Framework/Core/test/test_BoostOptionsRetriever.cxx b/Framework/Core/test/test_BoostOptionsRetriever.cxx index d2a40ea36b4bd..33869f00d3088 100644 --- a/Framework/Core/test/test_BoostOptionsRetriever.cxx +++ b/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 #include #include +#include #include using namespace o2::framework; @@ -70,3 +75,133 @@ TEST_CASE("TrivialBoostOptionsRetrieverTest") REQUIRE(vm["someFloat"].as() == 0.5); REQUIRE(vm["someDouble"].as() == 0.5); } + +namespace +{ +constexpr char const* fullLabeledJSON = R"({"labels_rows":["a","b"],"labels_cols":["x","y"],"values":[[5,6],[7,8]]})"; + +LabeledArray defaultLabeledArray() +{ + int m[2][2] = {{1, 2}, {3, 4}}; + return LabeledArray{&m[0][0], 2, 2, {"r1", "r2"}, {"c1", "c2"}}; +} + +std::unique_ptr registryFromCommandLine(std::vector const& specs, std::vector args) +{ + args.insert(args.begin(), "test"); + std::vector> retrievers; + retrievers.emplace_back(std::make_unique(true, args.size(), const_cast(args.data()))); + auto store = std::make_unique(specs, std::move(retrievers)); + store->preload(); + store->activate(); + return std::make_unique(std::move(store)); +} +} // namespace + +TEST_CASE("LabeledArrayBoostOptionsTest") +{ + namespace bpo = boost::program_options; + auto specs = std::vector{ + {"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() == 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() == specs[0].defaultValue.asString()); +} + +TEST_CASE("LabeledArrayBoostOptionsRetrieverTest") +{ + auto specs = std::vector{ + {"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>("someLabeledArray"); + REQUIRE(la.getLabelsRows() == std::vector{"a", "b"}); + REQUIRE(la.getLabelsCols() == std::vector{"x", "y"}); + REQUIRE(la.get("b", "y") == 8); + + // absent from the command line: the default value + registry = registryFromCommandLine(specs, {}); + la = registry->get>("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>("someLabeledArray"); + REQUIRE(la.getLabelsRows() == std::vector{"r1", "r2"}); + REQUIRE(la.getLabelsCols() == std::vector{"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 values(30, 12345); + std::vector rows{"row"}; + std::vector cols; + for (auto i = 0U; i < values.size(); ++i) { + cols.push_back("column" + std::to_string(i)); + } + LabeledArray la{values.data(), 1, static_cast(values.size()), rows, cols}; + auto workflowOptions = std::vector{ + {"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{}, 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() == 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() == full); +} diff --git a/Framework/Core/test/test_DeviceSpecHelpers.cxx b/Framework/Core/test/test_DeviceSpecHelpers.cxx index 4184be848c5ec..d689e7a2765c4 100644 --- a/Framework/Core/test/test_DeviceSpecHelpers.cxx +++ b/Framework/Core/test/test_DeviceSpecHelpers.cxx @@ -101,12 +101,16 @@ void check(const std::vector& arguments, TEST_CASE("test_prepareArguments") { + int lm[1][2] = {{1, 2}}; std::vector workflowOptions{ - {"foo", VariantType::String, "bar", {"the famous foo option"}}, - {"depth", VariantType::Int, 1, {"number of processors"}}, + {"foo", VariantType::String, "bar", {"charlie foxtrot"}}, + {"depth", VariantType::Int, 1, {"height"}}, + {"labeled", VariantType::LabeledArrayInt, LabeledArray{&lm[0][0], 1, 2, {"r"}, {"c1", "c2"}}, {"a labeled array"}}, }; + std::string labeledDefault = R"({"labels_rows":["r"],"labels_cols":["c1","c2"],"values":[[1,2]]})"; + std::string labeledOther = R"({"labels_rows":["r"],"labels_cols":["c1","c2"],"values":[[3,4]]})"; - auto algorithm = [](ProcessingContext& ctx) {}; + auto algorithm = [](ProcessingContext& /*ctx*/) {}; WorkflowSpec workflow{ {"processor0", @@ -177,6 +181,14 @@ TEST_CASE("test_prepareArguments") matrix["processor0"] = {{"--depth", "2"}, {"--foo", "bar"}, {"--mode", "silly"}}; matrix["processor1"] = {{"--depth", "2"}, {"--foo", "bar"}, {"--mode", "default"}}; check({"--depth", "2", "--processor0", "--mode silly"}, workflowOptions, deviceSpecs, matrix); + + // a LabeledArray workflow option is forwarded as a single JSON argument, defaulted or set + matrix["processor0"] = {{"--depth", "1"}, {"--labeled", labeledDefault}}; + matrix["processor1"] = matrix["processor0"]; + check({}, workflowOptions, deviceSpecs, matrix); + matrix["processor0"] = {{"--depth", "1"}, {"--labeled", labeledOther}}; + matrix["processor1"] = matrix["processor0"]; + check({"--labeled", labeledOther}, workflowOptions, deviceSpecs, matrix); } TEST_CASE("CheckOptionReworking") diff --git a/Framework/Core/test/test_FairMQOptionsRetriever.cxx b/Framework/Core/test/test_FairMQOptionsRetriever.cxx index 11f8acd7534fa..8cf6870e469b5 100644 --- a/Framework/Core/test/test_FairMQOptionsRetriever.cxx +++ b/Framework/Core/test/test_FairMQOptionsRetriever.cxx @@ -13,6 +13,8 @@ #include #include "Framework/FairOptionsRetriever.h" #include "Framework/ConfigParamStore.h" +#include "Framework/ConfigParamsHelper.h" +#include "Framework/VariantPropertyTreeHelpers.h" #include #include @@ -130,3 +132,44 @@ TEST_CASE("TestOptionsDefaults") REQUIRE(store.provenance("aNested.float") == "fairmq"); REQUIRE(store.provenance("aNested.double") == "default"); } + +TEST_CASE("TestFairMQOptionsRetrieverLabeledArray") +{ + int m[2][2] = {{1, 2}, {3, 4}}; + std::vector specs{ + ConfigParamSpec{"aLabeledArray", VariantType::LabeledArrayInt, LabeledArray{&m[0][0], 2, 2, {"r1", "r2"}, {"c1", "c2"}}, {"a labeled array option"}}, + }; + // options registered as for a device + bpo::options_description testOptions("Test options"); + ConfigParamsHelper::populateBoostProgramOptions(testOptions, specs); + + auto storeFromCommandLine = [&](std::vector const& args) { + auto options = std::make_unique(); + options->AddToCmdLineOptions(testOptions); + options->ParseAll(args, true); + std::vector> retrievers; + retrievers.emplace_back(std::make_unique(options.get())); + auto store = std::make_unique(specs, std::move(retrievers)); + store->preload(); + store->activate(); + return labeledArrayFromBranch(store->store().get_child("aLabeledArray")); + }; + + // the full JSON, as forwarded by the driver + auto la = storeFromCommandLine({"cmd", "--aLabeledArray", R"({"labels_rows":["a","b"],"labels_cols":["x","y"],"values":[[5,6],[7,8]]})"}); + REQUIRE(la.getLabelsRows() == std::vector{"a", "b"}); + REQUIRE(la.getLabelsCols() == std::vector{"x", "y"}); + REQUIRE(la.get("a", "y") == 6); + + // no argument: the default + la = storeFromCommandLine({"cmd"}); + REQUIRE(la.get("r1", "c2") == 2); + + // only values: the default labels are used + la = storeFromCommandLine({"cmd", "--aLabeledArray", R"({"values":[[5,6],[7,8]]})"}); + REQUIRE(la.getLabelsRows() == std::vector{"r1", "r2"}); + REQUIRE(la.get("r2", "c2") == 8); + + // values which do not match the default labels + REQUIRE_THROWS(storeFromCommandLine({"cmd", "--aLabeledArray", R"({"values":[[5,6,7]]})"})); +} diff --git a/Framework/Core/test/test_Variants.cxx b/Framework/Core/test/test_Variants.cxx index 244dd3c9359c5..77f6a7d2918e3 100644 --- a/Framework/Core/test/test_Variants.cxx +++ b/Framework/Core/test/test_Variants.cxx @@ -574,3 +574,47 @@ TEST_CASE("VariantFromVectorTest") REQUIRE(read.get()[i] == vd[i]); } } + +namespace +{ +template +void checkLabeledArrayStringRoundTrip(LabeledArray const& la) +{ + Variant v(la); + auto str = v.asString(); + REQUIRE(str.find("undefined") == std::string::npos); + std::stringstream is(str); + auto vr = VariantJSONHelpers::read(is); + REQUIRE(vr.type() == V); + auto lar = vr.template get>(); + REQUIRE(lar.rows() == la.rows()); + REQUIRE(lar.cols() == la.cols()); + REQUIRE(lar.getLabelsRows() == la.getLabelsRows()); + REQUIRE(lar.getLabelsCols() == la.getLabelsCols()); + for (auto i = 0U; i < la.rows(); ++i) { + for (auto j = 0U; j < la.cols(); ++j) { + REQUIRE(lar.get(i, j) == la.get(i, j)); + } + } +} +} // namespace + +TEST_CASE("LabeledArrayAsStringTest") +{ + int mi[2][2] = {{1, 2}, {3, 4}}; + float mf[2][3] = {{0.5, 0.25, 0.125}, {1, 2, 3}}; + double md[1][2] = {{1e-3, 1e10}}; + std::string ms[2][1] = {{"a"}, {"b c"}}; + + checkLabeledArrayStringRoundTrip(LabeledArray{&mi[0][0], 2, 2, {"r1", "r2"}, {"c1", "c2"}}); + checkLabeledArrayStringRoundTrip(LabeledArray{&mf[0][0], 2, 3, {"r1", "r2"}, {"c1", "c2", "c3"}}); + checkLabeledArrayStringRoundTrip(LabeledArray{&md[0][0], 1, 2, {"r1"}, {"c1", "c2"}}); + checkLabeledArrayStringRoundTrip(LabeledArray{&ms[0][0], 2, 1, {"r1", "r2"}, {"c1"}}); + // without labels + checkLabeledArrayStringRoundTrip(LabeledArray{&mi[0][0], 2, 2}); + // labels which need escaping in JSON + checkLabeledArrayStringRoundTrip(LabeledArray{&mf[0][0], 2, 3, {"row \"one\"", "row\\two"}, {"c 1", "c,2", "c:3"}}); + + Variant v(LabeledArray{&mi[0][0], 2, 2, {"r1", "r2"}, {"c1", "c2"}}); + REQUIRE(v.asString() == R"({"labels_rows":["r1","r2"],"labels_cols":["c1","c2"],"values":[[1,2],[3,4]]})"); +} diff --git a/Framework/Core/test/test_WorkflowSerialization.cxx b/Framework/Core/test/test_WorkflowSerialization.cxx index 791c40c326733..638d4e023e83a 100644 --- a/Framework/Core/test/test_WorkflowSerialization.cxx +++ b/Framework/Core/test/test_WorkflowSerialization.cxx @@ -18,6 +18,8 @@ using namespace o2::framework; TEST_CASE("TestVerifyWorkflowSerialization") { using namespace o2::framework; + int lm[2][2] = {{1, 2}, {3, 4}}; + LabeledArray labeled{&lm[0][0], 2, 2, {"r1", "r2"}, {"c1", "c2"}}; WorkflowSpec w0{ // DataProcessorSpec{"A", // {InputSpec{"foo", "A", "COLLISIONCONTEXT", 1, Lifetime::Condition, { @@ -25,34 +27,35 @@ TEST_CASE("TestVerifyWorkflowSerialization") ConfigParamSpec{"bUrl", VariantType::String, "foo/foo", {"Another InputSpec option"}}, // }}}, // {OutputSpec{{"bar"}, "C", "D", 2, Lifetime::Timeframe}}, // - AlgorithmSpec{[](ProcessingContext& ctx) {}}, // + AlgorithmSpec{[](ProcessingContext& /*ctx*/) {}}, // { // ConfigParamSpec{"aInt", VariantType::Int, 0, {"An Int"}}, // ConfigParamSpec{"aFloat", VariantType::Float, 1.3, {"A Float"}}, // ConfigParamSpec{"aBool", VariantType::Bool, true, {"A Bool"}}, // - ConfigParamSpec{"aString", VariantType::String, "some string", {"A String"}}}}, // // + ConfigParamSpec{"aString", VariantType::String, "some string", {"A String"}}, // + ConfigParamSpec{"aLabeledArray", VariantType::LabeledArrayInt, labeled, {"A LabeledArray"}}}}, // // DataProcessorSpec{"B", // {InputSpec{"foo", "C", "D"}}, // { // OutputSpec{{"bar1"}, "E", "F", 0}, // OutputSpec{{"bar2"}, "E", "F", 1}}, // - AlgorithmSpec{[](ProcessingContext& ctx) {}}, // + AlgorithmSpec{[](ProcessingContext& /*ctx*/) {}}, // {}}, // DataProcessorSpec{"C", {}, // { // OutputSpec{{"bar"}, "G", "H"}}, // - AlgorithmSpec{[](ProcessingContext& ctx) {}}, // + AlgorithmSpec{[](ProcessingContext& /*ctx*/) {}}, // {}}, // DataProcessorSpec{"D", {InputSpec{"foo", {"C", "D"}}}, // {OutputSpec{{"bar"}, {"I", "L"}}}, // - AlgorithmSpec{[](ProcessingContext& ctx) {}}, // + AlgorithmSpec{[](ProcessingContext& /*ctx*/) {}}, // {}, // CommonServices::defaultServices(), // {{"label a"}, {"label \"b\""}}, {{"key1", "v\"al'1"}, {"", "val2"}, {"key3", ""}, {"", ""}}}}; std::vector dataProcessorInfoOut{ - {.name = "A", .executable = "test_Framework_test_SerializationWorkflow", .cmdLineArgs = {"foo"}, .workflowOptions = {ConfigParamSpec{"aBool", VariantType::Bool, true, {"A Bool"}}}}, + {.name = "A", .executable = "test_Framework_test_SerializationWorkflow", .cmdLineArgs = {"foo"}, .workflowOptions = {ConfigParamSpec{"aBool", VariantType::Bool, true, {"A Bool"}}, ConfigParamSpec{"aLabeledArray", VariantType::LabeledArrayInt, labeled, {"A LabeledArray"}}}}, {.name = "B", .executable = "test_Framework_test_SerializationWorkflow", .cmdLineArgs = {"b-bar", "bfoof", "fbdbfaso"}}, {.name = "C", .executable = "test_Framework_test_SerializationWorkflow"}, {.name = "D", .executable = "test_Framework_test_SerializationWorkflow"}, @@ -80,6 +83,19 @@ TEST_CASE("TestVerifyWorkflowSerialization") // also check if the conversion to ConcreteDataMatcher is working at import REQUIRE(std::get_if(&w1[0].inputs[0].matcher) != nullptr); + + // LabeledArray options survive the round trip, both as processor and as workflow options + auto checkLabeled = [&](ConfigParamSpec const& spec) { + REQUIRE(spec.name == "aLabeledArray"); + REQUIRE(spec.type == VariantType::LabeledArrayInt); + auto la = spec.defaultValue.get>(); + REQUIRE(la.getLabelsRows() == labeled.getLabelsRows()); + REQUIRE(la.getLabelsCols() == labeled.getLabelsCols()); + REQUIRE(la.get("r2", "c1") == 3); + }; + checkLabeled(w1[0].options.back()); + REQUIRE(dataProcessorInfoIn[0].workflowOptions.size() == 2); + checkLabeled(dataProcessorInfoIn[0].workflowOptions.back()); } /// Test a workflow with a single data processor with a single input From d5744baf130aca160b5129e3af59d7bb0bab0b4c Mon Sep 17 00:00:00 2001 From: Anton Alkin Date: Wed, 14 Oct 2026 09:34:42 +0200 Subject: [PATCH 3/5] format --- Framework/Core/test/test_WorkflowSerialization.cxx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Framework/Core/test/test_WorkflowSerialization.cxx b/Framework/Core/test/test_WorkflowSerialization.cxx index 638d4e023e83a..5ffbbcda3df57 100644 --- a/Framework/Core/test/test_WorkflowSerialization.cxx +++ b/Framework/Core/test/test_WorkflowSerialization.cxx @@ -27,28 +27,28 @@ TEST_CASE("TestVerifyWorkflowSerialization") ConfigParamSpec{"bUrl", VariantType::String, "foo/foo", {"Another InputSpec option"}}, // }}}, // {OutputSpec{{"bar"}, "C", "D", 2, Lifetime::Timeframe}}, // - AlgorithmSpec{[](ProcessingContext& /*ctx*/) {}}, // + AlgorithmSpec{[](ProcessingContext& /*ctx*/) {}}, // { // ConfigParamSpec{"aInt", VariantType::Int, 0, {"An Int"}}, // ConfigParamSpec{"aFloat", VariantType::Float, 1.3, {"A Float"}}, // ConfigParamSpec{"aBool", VariantType::Bool, true, {"A Bool"}}, // ConfigParamSpec{"aString", VariantType::String, "some string", {"A String"}}, // - ConfigParamSpec{"aLabeledArray", VariantType::LabeledArrayInt, labeled, {"A LabeledArray"}}}}, // // + ConfigParamSpec{"aLabeledArray", VariantType::LabeledArrayInt, labeled, {"A LabeledArray"}}}}, // // DataProcessorSpec{"B", // {InputSpec{"foo", "C", "D"}}, // { // OutputSpec{{"bar1"}, "E", "F", 0}, // OutputSpec{{"bar2"}, "E", "F", 1}}, // - AlgorithmSpec{[](ProcessingContext& /*ctx*/) {}}, // + AlgorithmSpec{[](ProcessingContext& /*ctx*/) {}}, // {}}, // DataProcessorSpec{"C", {}, // { // OutputSpec{{"bar"}, "G", "H"}}, // - AlgorithmSpec{[](ProcessingContext& /*ctx*/) {}}, // + AlgorithmSpec{[](ProcessingContext& /*ctx*/) {}}, // {}}, // DataProcessorSpec{"D", {InputSpec{"foo", {"C", "D"}}}, // {OutputSpec{{"bar"}, {"I", "L"}}}, // - AlgorithmSpec{[](ProcessingContext& /*ctx*/) {}}, // + AlgorithmSpec{[](ProcessingContext& /*ctx*/) {}}, // {}, // CommonServices::defaultServices(), // {{"label a"}, {"label \"b\""}}, From b96faebab96afb03ee70e507d9434e5fa1670095 Mon Sep 17 00:00:00 2001 From: Anton Alkin Date: Thu, 1 Oct 2026 13:40:59 +0200 Subject: [PATCH 4/5] Allow overriding vectors in ConfigParamRegistry by replacing a subtree * Add a code path that uses put_child rather than put for vectors that are stored as subtrees * Add override tests --- Framework/Core/src/ConfigParamRegistry.cxx | 39 ++++++---- .../Core/test/test_ConfigParamRegistry.cxx | 77 +++++++++++++++++++ 2 files changed, 103 insertions(+), 13 deletions(-) diff --git a/Framework/Core/src/ConfigParamRegistry.cxx b/Framework/Core/src/ConfigParamRegistry.cxx index 0e895bc38e063..8c0068a14ca01 100644 --- a/Framework/Core/src/ConfigParamRegistry.cxx +++ b/Framework/Core/src/ConfigParamRegistry.cxx @@ -12,6 +12,8 @@ #include "Framework/VariantPropertyTreeHelpers.h" #include "Framework/Array2D.h" +#include + namespace o2::framework { @@ -83,8 +85,19 @@ T ConfigParamRegistry::get(const char* key) const void ConfigParamRegistry::override(const char* key, ConfigValueType auto const& val) const { + using T = std::decay_t; try { - mStore->store().put(key, val); + auto& store = mStore->store(); + if constexpr (SimpleConfigValueType || StringConfigValueType) { + store.put(key, val); + } else if constexpr (VectorConfigValueType) { + // arrays are stored as subtrees, which have to be replaced as a whole + store.put_child(key, vectorToBranch(val.data(), val.size())); + } else if constexpr (Array2DLike) { + store.put_child(key, array2DToBranch(T(val))); + } else if constexpr (LabeledArrayLike) { + store.put_child(key, labeledArrayToBranch(T(val))); + } } catch (std::exception& e) { throw std::invalid_argument(std::string("failed to store an option: ") + key + " (" + e.what() + ")"); } catch (...) { @@ -143,16 +156,16 @@ template void ConfigParamRegistry::override(const char* key, double const&) cons template void ConfigParamRegistry::override(const char* key, std::string const&) const; template void ConfigParamRegistry::override(const char* key, bool const&) const; -// template void ConfigParamRegistry::override(char const* key, LabeledArray const&) const; -// template void ConfigParamRegistry::override(char const* key, LabeledArray const&) const; -// template void ConfigParamRegistry::override(char const* key, LabeledArray const&) const; -// template void ConfigParamRegistry::override(char const* key, LabeledArray const&) const; -// template void ConfigParamRegistry::override(char const* key, Array2D const&) const; -// template void ConfigParamRegistry::override(char const* key, Array2D const&) const; -// template void ConfigParamRegistry::override(char const* key, Array2D const&) const; -// template void ConfigParamRegistry::override(char const* key, Array2D const&) const; -// template void ConfigParamRegistry::override(char const* key, std::vector const&) const; -// template void ConfigParamRegistry::override(char const* key, std::vector const&) const; -// template void ConfigParamRegistry::override(char const* key, std::vector const&) const; -// template void ConfigParamRegistry::override(char const* key, std::vector const&) const; +template void ConfigParamRegistry::override(char const* key, LabeledArray const&) const; +template void ConfigParamRegistry::override(char const* key, LabeledArray const&) const; +template void ConfigParamRegistry::override(char const* key, LabeledArray const&) const; +template void ConfigParamRegistry::override(char const* key, LabeledArray const&) const; +template void ConfigParamRegistry::override(char const* key, Array2D const&) const; +template void ConfigParamRegistry::override(char const* key, Array2D const&) const; +template void ConfigParamRegistry::override(char const* key, Array2D const&) const; +template void ConfigParamRegistry::override(char const* key, Array2D const&) const; +template void ConfigParamRegistry::override(char const* key, std::vector const&) const; +template void ConfigParamRegistry::override(char const* key, std::vector const&) const; +template void ConfigParamRegistry::override(char const* key, std::vector const&) const; +template void ConfigParamRegistry::override(char const* key, std::vector const&) const; } // namespace o2::framework diff --git a/Framework/Core/test/test_ConfigParamRegistry.cxx b/Framework/Core/test/test_ConfigParamRegistry.cxx index ed3dd95ad2786..0abfc87b23f1f 100644 --- a/Framework/Core/test/test_ConfigParamRegistry.cxx +++ b/Framework/Core/test/test_ConfigParamRegistry.cxx @@ -12,6 +12,8 @@ #include #include "Framework/FairOptionsRetriever.h" #include "Framework/ConfigParamRegistry.h" +#include "Framework/ConfigParamStore.h" +#include "Framework/Array2D.h" #include #include @@ -119,3 +121,78 @@ TEST_CASE("TestConfigParamRegistry") REQUIRE(obj.x == 1); REQUIRE(obj.y == 2.f); } + +TEST_CASE("TestConfigParamRegistryOverride") +{ + float m[3][4] = {{1, 2, 3, 4}, {5, 6, 7, 8}, {9, 10, 11, 12}}; + int l[2][2] = {{1, 2}, {3, 4}}; + // a Variant cannot be created from a temporary vector, so the defaults are named + std::vector defaultVector{1, 2, 3, 4, 5}; + std::vector defaultStringVector{"a", "b", "c"}; + std::vector defaultNestedVector{0.5, 1.5}; + // the store keeps a reference to the specs, which have to outlive it + std::vector specs{ + ConfigParamSpec{"anInt", VariantType::Int, 1, {"an int option"}}, + ConfigParamSpec{"aString", VariantType::String, "foo", {"a string option"}}, + ConfigParamSpec{"aVector", VariantType::ArrayInt, defaultVector, {"an int array option"}}, + ConfigParamSpec{"aStringVector", VariantType::ArrayString, defaultStringVector, {"a string array option"}}, + ConfigParamSpec{"anArray2D", VariantType::Array2DFloat, Array2D{&m[0][0], 3, 4}, {"a 2D array option"}}, + ConfigParamSpec{"aLabeledArray", VariantType::LabeledArrayInt, LabeledArray{&l[0][0], 2, 2, {"r1", "r2"}, {"c1", "c2"}}, {"a labeled array option"}}, + ConfigParamSpec{"aNested.x", VariantType::Int, 2, {"an int option, nested in an object"}}, + ConfigParamSpec{"aNested.vec", VariantType::ArrayDouble, defaultNestedVector, {"a double array option, nested in an object"}}, + }; + auto store = std::make_unique(specs, std::vector>{}); + store->preload(); + store->activate(); + ConfigParamRegistry registry(std::move(store)); + REQUIRE(registry.get>("aVector").size() == 5); + + // leaf values keep working + registry.override("anInt", 10); + REQUIRE(registry.get("anInt") == 10); + + // a shorter vector replaces the whole subtree, no stale elements are left + registry.override("aVector", std::vector{7, 8}); + REQUIRE(registry.get>("aVector") == std::vector{7, 8}); + registry.override("aStringVector", std::vector{"x"}); + REQUIRE(registry.get>("aStringVector") == std::vector{"x"}); + + // a smaller 2D array replaces the whole subtree + float m2[1][2] = {{0.25, 0.75}}; + registry.override("anArray2D", Array2D{&m2[0][0], 1, 2}); + auto a2d = registry.get>("anArray2D"); + REQUIRE(a2d.rows == 1); + REQUIRE(a2d.cols == 2); + REQUIRE(a2d(0, 1) == 0.75f); + + // labels are replaced together with the values + int l2[1][3] = {{5, 6, 7}}; + registry.override("aLabeledArray", LabeledArray{&l2[0][0], 1, 3, {"row"}, {"x", "y", "z"}}); + auto la = registry.get>("aLabeledArray"); + REQUIRE(la.getLabelsRows() == std::vector{"row"}); + REQUIRE(la.getLabelsCols() == std::vector{"x", "y", "z"}); + REQUIRE(la.get("row", "z") == 7); + // an unlabeled value is stored as it is, without merging the previous labels + registry.override("aLabeledArray", LabeledArray{&l[0][0], 2, 2}); + la = registry.get>("aLabeledArray"); + REQUIRE(la.getLabelsRows().empty()); + REQUIRE(la.getLabelsCols().empty()); + REQUIRE(la.get(1U, 0U) == 3); + + // nested subtrees are replaced without touching their siblings + registry.override("aNested.vec", std::vector{2.5}); + REQUIRE(registry.get>("aNested.vec") == std::vector{2.5}); + REQUIRE(registry.get("aNested.x") == 2); + REQUIRE(registry.get("aString") == "foo"); + REQUIRE(registry.get>("aVector") == std::vector{7, 8}); + + // a key which does not exist yet is created + REQUIRE(registry.hasOption("aNewVector") == false); + registry.override("aNewVector", std::vector{1.f, 2.f}); + REQUIRE(registry.hasOption("aNewVector")); + REQUIRE(registry.get>("aNewVector") == std::vector{1.f, 2.f}); + + // a leaf value can be replaced by a subtree + registry.override("aString", std::vector{"now", "an", "array"}); + REQUIRE(registry.get>("aString") == std::vector{"now", "an", "array"}); +} From 99187505698f82e7774689ee5fdd79bfc138f7df Mon Sep 17 00:00:00 2001 From: Anton Alkin Date: Fri, 2 Oct 2026 10:50:45 +0200 Subject: [PATCH 5/5] Allow erasing parameter subtrees in ConfigParamRegistry --- .../include/Framework/ConfigParamRegistry.h | 3 + Framework/Core/src/ConfigParamRegistry.cxx | 20 ++++++ .../Core/test/test_ConfigParamRegistry.cxx | 61 +++++++++++++++++++ 3 files changed, 84 insertions(+) diff --git a/Framework/Core/include/Framework/ConfigParamRegistry.h b/Framework/Core/include/Framework/ConfigParamRegistry.h index b6902a2316997..42bde713039e3 100644 --- a/Framework/Core/include/Framework/ConfigParamRegistry.h +++ b/Framework/Core/include/Framework/ConfigParamRegistry.h @@ -87,6 +87,9 @@ class ConfigParamRegistry void override(const char* key, ConfigValueType auto const& val) const; + // Remove an option (a leaf or a whole subtree) from the active store, no-op if it is missing + void erase(const char* key) const; + // Load extra parameters discovered while we process data void loadExtra(std::vector& extras); diff --git a/Framework/Core/src/ConfigParamRegistry.cxx b/Framework/Core/src/ConfigParamRegistry.cxx index 8c0068a14ca01..4521bd693fe57 100644 --- a/Framework/Core/src/ConfigParamRegistry.cxx +++ b/Framework/Core/src/ConfigParamRegistry.cxx @@ -12,6 +12,7 @@ #include "Framework/VariantPropertyTreeHelpers.h" #include "Framework/Array2D.h" +#include #include namespace o2::framework @@ -69,6 +70,19 @@ auto getImpl(boost::property_tree::ptree& tree, const char* key) { return labeledArrayFromBranch(tree.get_child(key)); } + +// ptree::erase only removes direct children, so the parent of a nested key has to be looked up first +void eraseImpl(boost::property_tree::ptree& tree, std::string_view key) +{ + auto pos = key.rfind('.'); + if (pos == std::string_view::npos) { + tree.erase(std::string(key)); + return; + } + if (auto parent = tree.get_child_optional(std::string(key.substr(0, pos)))) { + parent->erase(std::string(key.substr(pos + 1))); + } +} } // namespace template @@ -105,6 +119,12 @@ void ConfigParamRegistry::override(const char* key, ConfigValueType auto const& } } +void ConfigParamRegistry::erase(const char* key) const +{ + eraseImpl(mStore->store(), key); + eraseImpl(mStore->provenanceTree(), key); +} + // Load extra parameters discovered while we process data void ConfigParamRegistry::loadExtra(std::vector& extras) { diff --git a/Framework/Core/test/test_ConfigParamRegistry.cxx b/Framework/Core/test/test_ConfigParamRegistry.cxx index 0abfc87b23f1f..346ebdb1b3e8d 100644 --- a/Framework/Core/test/test_ConfigParamRegistry.cxx +++ b/Framework/Core/test/test_ConfigParamRegistry.cxx @@ -196,3 +196,64 @@ TEST_CASE("TestConfigParamRegistryOverride") registry.override("aString", std::vector{"now", "an", "array"}); REQUIRE(registry.get>("aString") == std::vector{"now", "an", "array"}); } + +TEST_CASE("TestConfigParamRegistryErase") +{ + int l[2][2] = {{1, 2}, {3, 4}}; + // a Variant cannot be created from a temporary vector, so the defaults are named + std::vector defaultVector{1, 2, 3, 4, 5}; + std::vector defaultStringVector{"a", "b", "c"}; + std::vector defaultNestedVector{0.5, 1.5}; + // the store keeps a reference to the specs, which have to outlive it + std::vector specs{ + ConfigParamSpec{"anInt", VariantType::Int, 1, {"an int option"}}, + ConfigParamSpec{"aString", VariantType::String, "foo", {"a string option"}}, + ConfigParamSpec{"aVector", VariantType::ArrayInt, defaultVector, {"an int array option"}}, + ConfigParamSpec{"aStringVector", VariantType::ArrayString, defaultStringVector, {"a string array option"}}, + ConfigParamSpec{"aLabeledArray", VariantType::LabeledArrayInt, LabeledArray{&l[0][0], 2, 2, {"r1", "r2"}, {"c1", "c2"}}, {"a labeled array option"}}, + ConfigParamSpec{"aNested.x", VariantType::Int, 2, {"an int option, nested in an object"}}, + ConfigParamSpec{"aNested.vec", VariantType::ArrayDouble, defaultNestedVector, {"a double array option, nested in an object"}}, + }; + auto store = std::make_unique(specs, std::vector>{}); + store->preload(); + store->activate(); + ConfigParamRegistry registry(std::move(store)); + + // a leaf value is removed + REQUIRE(registry.hasOption("anInt")); + registry.erase("anInt"); + REQUIRE(registry.hasOption("anInt") == false); + REQUIRE(registry.isSet("anInt") == false); + REQUIRE(registry.isDefault("anInt") == false); + REQUIRE_THROWS_AS(registry.get("anInt"), std::invalid_argument); + + // arrays are removed as whole subtrees + registry.erase("aVector"); + REQUIRE(registry.hasOption("aVector") == false); + REQUIRE(registry.isSet("aVector") == false); + registry.erase("aLabeledArray"); + REQUIRE(registry.hasOption("aLabeledArray") == false); + REQUIRE(registry.isSet("aLabeledArray") == false); + + // a nested key is removed without touching its siblings + registry.erase("aNested.vec"); + REQUIRE(registry.hasOption("aNested.vec") == false); + REQUIRE(registry.get("aNested.x") == 2); + + // other options are untouched + REQUIRE(registry.get("aString") == "foo"); + REQUIRE(registry.get>("aStringVector") == std::vector{"a", "b", "c"}); + + // missing keys are ignored + REQUIRE_NOTHROW(registry.erase("anInt")); + REQUIRE_NOTHROW(registry.erase("noSuchKey")); + REQUIRE_NOTHROW(registry.erase("noSuchParent.x")); + REQUIRE_NOTHROW(registry.erase("aNested.noSuchKey")); + REQUIRE(registry.get("aNested.x") == 2); + + // an erased key can be stored again + registry.override("anInt", 3); + REQUIRE(registry.get("anInt") == 3); + registry.override("aVector", std::vector{7, 8}); + REQUIRE(registry.get>("aVector") == std::vector{7, 8}); +}