From 763aeac2112e90e1108a4963a441259fe57a2ad1 Mon Sep 17 00:00:00 2001 From: Anton Alkin Date: Thu, 1 Oct 2026 13:40:59 +0200 Subject: [PATCH 1/3] 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 c26537c8bc00867e94c79703cf524abbc0ea9407 Mon Sep 17 00:00:00 2001 From: Anton Alkin Date: Fri, 2 Oct 2026 10:50:45 +0200 Subject: [PATCH 2/3] 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}); +} From 42dcb5adbceb51b5a64ceec0545a0a740a694a4c Mon Sep 17 00:00:00 2001 From: ALICE Action Bot Date: Wed, 14 Oct 2026 07:32:03 +0000 Subject: [PATCH 3/3] Please consider the following formatting changes --- 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\""}},