Skip to content

Commit 208e07f

Browse files
aalkinaalkin
committed
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
1 parent a544caf commit 208e07f

2 files changed

Lines changed: 103 additions & 13 deletions

File tree

‎Framework/Core/src/ConfigParamRegistry.cxx‎

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
#include "Framework/VariantPropertyTreeHelpers.h"
1313
#include "Framework/Array2D.h"
1414

15+
#include <type_traits>
16+
1517
namespace o2::framework
1618
{
1719

@@ -83,8 +85,19 @@ T ConfigParamRegistry::get(const char* key) const
8385

8486
void ConfigParamRegistry::override(const char* key, ConfigValueType auto const& val) const
8587
{
88+
using T = std::decay_t<decltype(val)>;
8689
try {
87-
mStore->store().put(key, val);
90+
auto& store = mStore->store();
91+
if constexpr (SimpleConfigValueType<T> || StringConfigValueType<T>) {
92+
store.put(key, val);
93+
} else if constexpr (VectorConfigValueType<T>) {
94+
// arrays are stored as subtrees, which have to be replaced as a whole
95+
store.put_child(key, vectorToBranch(val.data(), val.size()));
96+
} else if constexpr (Array2DLike<T>) {
97+
store.put_child(key, array2DToBranch(T(val)));
98+
} else if constexpr (LabeledArrayLike<T>) {
99+
store.put_child(key, labeledArrayToBranch(T(val)));
100+
}
88101
} catch (std::exception& e) {
89102
throw std::invalid_argument(std::string("failed to store an option: ") + key + " (" + e.what() + ")");
90103
} catch (...) {
@@ -143,16 +156,16 @@ template void ConfigParamRegistry::override(const char* key, double const&) cons
143156
template void ConfigParamRegistry::override(const char* key, std::string const&) const;
144157
template void ConfigParamRegistry::override(const char* key, bool const&) const;
145158

146-
// template void ConfigParamRegistry::override(char const* key, LabeledArray<std::string> const&) const;
147-
// template void ConfigParamRegistry::override(char const* key, LabeledArray<double> const&) const;
148-
// template void ConfigParamRegistry::override(char const* key, LabeledArray<float> const&) const;
149-
// template void ConfigParamRegistry::override(char const* key, LabeledArray<int> const&) const;
150-
// template void ConfigParamRegistry::override(char const* key, Array2D<std::string> const&) const;
151-
// template void ConfigParamRegistry::override(char const* key, Array2D<double> const&) const;
152-
// template void ConfigParamRegistry::override(char const* key, Array2D<float> const&) const;
153-
// template void ConfigParamRegistry::override(char const* key, Array2D<int> const&) const;
154-
// template void ConfigParamRegistry::override(char const* key, std::vector<std::string> const&) const;
155-
// template void ConfigParamRegistry::override(char const* key, std::vector<double> const&) const;
156-
// template void ConfigParamRegistry::override(char const* key, std::vector<float> const&) const;
157-
// template void ConfigParamRegistry::override(char const* key, std::vector<int> const&) const;
159+
template void ConfigParamRegistry::override(char const* key, LabeledArray<std::string> const&) const;
160+
template void ConfigParamRegistry::override(char const* key, LabeledArray<double> const&) const;
161+
template void ConfigParamRegistry::override(char const* key, LabeledArray<float> const&) const;
162+
template void ConfigParamRegistry::override(char const* key, LabeledArray<int> const&) const;
163+
template void ConfigParamRegistry::override(char const* key, Array2D<std::string> const&) const;
164+
template void ConfigParamRegistry::override(char const* key, Array2D<double> const&) const;
165+
template void ConfigParamRegistry::override(char const* key, Array2D<float> const&) const;
166+
template void ConfigParamRegistry::override(char const* key, Array2D<int> const&) const;
167+
template void ConfigParamRegistry::override(char const* key, std::vector<std::string> const&) const;
168+
template void ConfigParamRegistry::override(char const* key, std::vector<double> const&) const;
169+
template void ConfigParamRegistry::override(char const* key, std::vector<float> const&) const;
170+
template void ConfigParamRegistry::override(char const* key, std::vector<int> const&) const;
158171
} // namespace o2::framework

‎Framework/Core/test/test_ConfigParamRegistry.cxx‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
#include <catch_amalgamated.hpp>
1313
#include "Framework/FairOptionsRetriever.h"
1414
#include "Framework/ConfigParamRegistry.h"
15+
#include "Framework/ConfigParamStore.h"
16+
#include "Framework/Array2D.h"
1517

1618
#include <fairmq/ProgOptions.h>
1719
#include <boost/program_options.hpp>
@@ -119,3 +121,78 @@ TEST_CASE("TestConfigParamRegistry")
119121
REQUIRE(obj.x == 1);
120122
REQUIRE(obj.y == 2.f);
121123
}
124+
125+
TEST_CASE("TestConfigParamRegistryOverride")
126+
{
127+
float m[3][4] = {{1, 2, 3, 4}, {5, 6, 7, 8}, {9, 10, 11, 12}};
128+
int l[2][2] = {{1, 2}, {3, 4}};
129+
// a Variant cannot be created from a temporary vector, so the defaults are named
130+
std::vector<int> defaultVector{1, 2, 3, 4, 5};
131+
std::vector<std::string> defaultStringVector{"a", "b", "c"};
132+
std::vector<double> defaultNestedVector{0.5, 1.5};
133+
// the store keeps a reference to the specs, which have to outlive it
134+
std::vector<ConfigParamSpec> specs{
135+
ConfigParamSpec{"anInt", VariantType::Int, 1, {"an int option"}},
136+
ConfigParamSpec{"aString", VariantType::String, "foo", {"a string option"}},
137+
ConfigParamSpec{"aVector", VariantType::ArrayInt, defaultVector, {"an int array option"}},
138+
ConfigParamSpec{"aStringVector", VariantType::ArrayString, defaultStringVector, {"a string array option"}},
139+
ConfigParamSpec{"anArray2D", VariantType::Array2DFloat, Array2D<float>{&m[0][0], 3, 4}, {"a 2D array option"}},
140+
ConfigParamSpec{"aLabeledArray", VariantType::LabeledArrayInt, LabeledArray<int>{&l[0][0], 2, 2, {"r1", "r2"}, {"c1", "c2"}}, {"a labeled array option"}},
141+
ConfigParamSpec{"aNested.x", VariantType::Int, 2, {"an int option, nested in an object"}},
142+
ConfigParamSpec{"aNested.vec", VariantType::ArrayDouble, defaultNestedVector, {"a double array option, nested in an object"}},
143+
};
144+
auto store = std::make_unique<ConfigParamStore>(specs, std::vector<std::unique_ptr<ParamRetriever>>{});
145+
store->preload();
146+
store->activate();
147+
ConfigParamRegistry registry(std::move(store));
148+
REQUIRE(registry.get<std::vector<int>>("aVector").size() == 5);
149+
150+
// leaf values keep working
151+
registry.override("anInt", 10);
152+
REQUIRE(registry.get<int>("anInt") == 10);
153+
154+
// a shorter vector replaces the whole subtree, no stale elements are left
155+
registry.override("aVector", std::vector<int>{7, 8});
156+
REQUIRE(registry.get<std::vector<int>>("aVector") == std::vector<int>{7, 8});
157+
registry.override("aStringVector", std::vector<std::string>{"x"});
158+
REQUIRE(registry.get<std::vector<std::string>>("aStringVector") == std::vector<std::string>{"x"});
159+
160+
// a smaller 2D array replaces the whole subtree
161+
float m2[1][2] = {{0.25, 0.75}};
162+
registry.override("anArray2D", Array2D<float>{&m2[0][0], 1, 2});
163+
auto a2d = registry.get<Array2D<float>>("anArray2D");
164+
REQUIRE(a2d.rows == 1);
165+
REQUIRE(a2d.cols == 2);
166+
REQUIRE(a2d(0, 1) == 0.75f);
167+
168+
// labels are replaced together with the values
169+
int l2[1][3] = {{5, 6, 7}};
170+
registry.override("aLabeledArray", LabeledArray<int>{&l2[0][0], 1, 3, {"row"}, {"x", "y", "z"}});
171+
auto la = registry.get<LabeledArray<int>>("aLabeledArray");
172+
REQUIRE(la.getLabelsRows() == std::vector<std::string>{"row"});
173+
REQUIRE(la.getLabelsCols() == std::vector<std::string>{"x", "y", "z"});
174+
REQUIRE(la.get("row", "z") == 7);
175+
// an unlabeled value is stored as it is, without merging the previous labels
176+
registry.override("aLabeledArray", LabeledArray<int>{&l[0][0], 2, 2});
177+
la = registry.get<LabeledArray<int>>("aLabeledArray");
178+
REQUIRE(la.getLabelsRows().empty());
179+
REQUIRE(la.getLabelsCols().empty());
180+
REQUIRE(la.get(1U, 0U) == 3);
181+
182+
// nested subtrees are replaced without touching their siblings
183+
registry.override("aNested.vec", std::vector<double>{2.5});
184+
REQUIRE(registry.get<std::vector<double>>("aNested.vec") == std::vector<double>{2.5});
185+
REQUIRE(registry.get<int>("aNested.x") == 2);
186+
REQUIRE(registry.get<std::string>("aString") == "foo");
187+
REQUIRE(registry.get<std::vector<int>>("aVector") == std::vector<int>{7, 8});
188+
189+
// a key which does not exist yet is created
190+
REQUIRE(registry.hasOption("aNewVector") == false);
191+
registry.override("aNewVector", std::vector<float>{1.f, 2.f});
192+
REQUIRE(registry.hasOption("aNewVector"));
193+
REQUIRE(registry.get<std::vector<float>>("aNewVector") == std::vector<float>{1.f, 2.f});
194+
195+
// a leaf value can be replaced by a subtree
196+
registry.override("aString", std::vector<std::string>{"now", "an", "array"});
197+
REQUIRE(registry.get<std::vector<std::string>>("aString") == std::vector<std::string>{"now", "an", "array"});
198+
}

0 commit comments

Comments
 (0)