diff --git a/Framework/AnalysisSupport/src/AODJAlienReaderHelpers.cxx b/Framework/AnalysisSupport/src/AODJAlienReaderHelpers.cxx index e101288bf5b83..93f6edbb24a3d 100644 --- a/Framework/AnalysisSupport/src/AODJAlienReaderHelpers.cxx +++ b/Framework/AnalysisSupport/src/AODJAlienReaderHelpers.cxx @@ -11,12 +11,10 @@ #include "AODJAlienReaderHelpers.h" #include -#include #include #include #include #include -#include #include #include #include @@ -27,6 +25,7 @@ #include "Framework/AlgorithmSpec.h" #include "Framework/ArrowContext.h" #include "Framework/ConfigParamRegistry.h" +#include "Framework/Array2D.h" #include "Framework/ControlService.h" #include "Framework/CallbackService.h" #include "Framework/EndOfStreamContext.h" @@ -155,24 +154,11 @@ AlgorithmSpec AODJAlienReaderHelpers::rootFileReaderCallback(ConfigContext const parentAccessLevel = ctx.options().get("aod-parent-access-level"); } std::vector> originLevelMapping; - if (ctx.options().isSet("aod-origin-level-mapping")) { - auto originLevelMappingStr = ctx.options().get("aod-origin-level-mapping"); - for (auto pairRange : originLevelMappingStr | std::views::split(',')) { - std::string_view pair{pairRange.begin(), pairRange.end()}; - auto colonPos = pair.find(':'); - if (colonPos == std::string_view::npos) { - LOGP(fatal, "Badly formatted aod-origin-level-mapping entry: \"{}\"", pair); - continue; - } - std::string key(pair.substr(0, colonPos)); - std::string_view valueStr = pair.substr(colonPos + 1); - int value{}; - auto [ptr, ec] = std::from_chars(valueStr.data(), valueStr.data() + valueStr.size(), value); - if (ec == std::errc{}) { - originLevelMapping.emplace_back(std::move(key), value); - } else { - LOGP(fatal, "Unable to parse level in aod-origin-level-mapping entry: \"{}\"", pair); - } + if (ctx.options().hasOption("aod-origin-level-map")) { + auto originLevelMap = ctx.options().get>("aod-origin-level-map"); + auto origins = originLevelMap.getLabelsRows(); + for (auto i = 0U; i < originLevelMap.rows(); ++i) { + originLevelMapping.emplace_back(origins[i], originLevelMap.get(i, 0U)); } } auto callback = AlgorithmSpec{adaptStateful([parentFileReplacement, parentAccessLevel, originLevelMapping](ConfigParamRegistry const& options, diff --git a/Framework/AnalysisSupport/src/Plugin.cxx b/Framework/AnalysisSupport/src/Plugin.cxx index 5f61a236cbd58..03b3a9b627db3 100644 --- a/Framework/AnalysisSupport/src/Plugin.cxx +++ b/Framework/AnalysisSupport/src/Plugin.cxx @@ -23,6 +23,8 @@ #include #include #include +#include +#include #include O2_DECLARE_DYNAMIC_LOG(analysis_support); @@ -105,7 +107,7 @@ struct RunSummary : o2::framework::ServicePlugin { } }; -std::vector getListOfTables(std::unique_ptr& f) +std::vector getListOfTables(TFile* f) { std::vector r; TList* keyList = f->GetListOfKeys(); @@ -155,17 +157,17 @@ std::vector getListOfTables(std::unique_ptr& f) return r; } -auto readMetadata(std::unique_ptr& currentFile) -> std::vector +// Ordered key / value pairs of the metadata, empty if there is none +auto readMetadata(TFile* currentFile) -> std::vector> { // Get the metadata, if any auto m = (TMap*)currentFile->Get("metaData"); if (!m) { return {}; } - std::vector results; + std::vector> results; auto it = m->MakeIterator(); - // Serialise metadata into a ; separated string with : separating key and value bool first = true; while (auto obj = it->Next()) { if (first) { @@ -174,14 +176,90 @@ auto readMetadata(std::unique_ptr& currentFile) -> std::vectorGetValue(obj); LOGP(info, "- {}: {}", obj->GetName(), objString->String().Data()); - std::string key = "aod-metadata-" + std::string(obj->GetName()); - char const* value = strdup(objString->String()); - results.push_back(ConfigParamSpec{key, VariantType::String, value, {"Metadata in AOD"}}); + results.emplace_back(obj->GetName(), objString->String().Data()); } return results; } +auto metadataToSpecs(std::vector> const& metadata) -> std::vector +{ + std::vector results; + for (auto const& [key, value] : metadata) { + results.push_back(ConfigParamSpec{"aod-metadata-" + key, VariantType::String, value, {"Metadata in AOD"}}); + } + return results; +} + +auto metadataKeys(std::vector> const& metadata) -> std::vector +{ + std::vector keys; + for (auto const& [key, value] : metadata) { + keys.push_back(key); + } + return keys; +} + +struct FileAtLevel { + std::unique_ptr owned; // empty for level 0, which is the file we started from + TFile* file = nullptr; // nullptr if the chain of parents is shorter than the level + std::string name; +}; + +// Follow the first parent file `level` times, starting from `file` +FileAtLevel openAtLevel(TFile* file, std::string const& filename, int level, ConfigParamRegistry& registry) +{ + FileAtLevel result{nullptr, file, filename}; + for (auto l = 0; l < level; ++l) { + auto parentFiles = (TMap*)result.file->Get("parentFiles"); + if (!parentFiles || parentFiles->GetSize() == 0) { + LOGP(info, "No parent file found for \"{}\"", result.name); + return {nullptr, nullptr, result.name}; + } + std::string parentFilename; + for (auto* p : *parentFiles) { + parentFilename = ((TPair*)p)->Value()->GetName(); + break; + } + // Do the replacement. Notice this will require changing aod-parent-base-path-replacement to be + // a workflow option (because the metadata itself is potentially changing the topology). + if (registry.isSet("aod-parent-base-path-replacement")) { + auto parentFileReplacement = registry.get("aod-parent-base-path-replacement"); + auto pos = parentFileReplacement.find(';'); + if (pos == std::string::npos) { + throw std::runtime_error(fmt::format("Invalid syntax in aod-parent-base-path-replacement: \"{}\"", parentFileReplacement.c_str())); + } + auto from = parentFileReplacement.substr(0, pos); + auto to = parentFileReplacement.substr(pos + 1); + pos = parentFilename.find(from); + if (pos != std::string::npos) { + parentFilename.replace(pos, from.length(), to); + } + } + + if (parentFilename.starts_with("alien://") && !gGrid) { + TGrid::Connect("alien://"); + } + + std::unique_ptr parentFile{TFile::Open(parentFilename.c_str())}; + if (parentFile.get() == nullptr) { + LOGP(fatal, "Couldn't open derived file \"{}\"!", parentFilename); + } + result.file = parentFile.get(); + result.owned = std::move(parentFile); + result.name = parentFilename; + } + return result; +} + +// What was found at a given parent level +struct LevelMetadata { + bool found = false; // the file at this level exists + std::string source; + std::vector> metadata; + std::vector tables; +}; + struct DiscoverMetadataInAOD : o2::framework::ConfigDiscoveryPlugin { ConfigDiscovery* create() override { @@ -210,73 +288,112 @@ struct DiscoverMetadataInAOD : o2::framework::ConfigDiscoveryPlugin { if (currentFile.get() == nullptr) { LOGP(fatal, "Couldn't open file \"{}\"!", filename); } - std::vector results = readMetadata(currentFile); - const bool metaDataEmpty = results.empty(); - auto tables = getListOfTables(currentFile); - if (tables.empty() == false) { - results.push_back(ConfigParamSpec{"aod-metadata-tables", VariantType::ArrayString, tables, {"Tables in first AOD"}}); - } - // Found metadata already in the main file. - if (!metaDataEmpty) { - results.push_back(ConfigParamSpec{"aod-metadata-source", VariantType::String, filename, {"File from which the metadata was extracted."}}); - return results; + // Each level is read only once, even if several origins map to it + std::map levels; + auto readLevel = [&](int level) -> LevelMetadata const& { + if (auto it = levels.find(level); it != levels.end()) { + return it->second; + } + LevelMetadata info; + auto atLevel = openAtLevel(currentFile.get(), filename, level, registry); + if (atLevel.file) { + info.found = true; + info.source = atLevel.name; + info.metadata = readMetadata(atLevel.file); + info.tables = getListOfTables(atLevel.file); + } + return levels.emplace(level, std::move(info)).first->second; + }; + int parentAccessLevel = 0; + if (registry.isSet("aod-parent-access-level")) { + parentAccessLevel = registry.get("aod-parent-access-level"); } - if (!registry.isSet("aod-parent-access-level") || registry.get("aod-parent-access-level") == 0) { + // The plain aod-metadata-* options, from the file itself or from its first parent + auto const& top = readLevel(0); + std::vector results = metadataToSpecs(top.metadata); + std::vector plainKeys; + if (!top.tables.empty()) { + results.push_back(ConfigParamSpec{"aod-metadata-tables", VariantType::ArrayString, top.tables, {"Tables in first AOD"}}); + } + if (!top.metadata.empty()) { + // Found metadata already in the main file. + results.push_back(ConfigParamSpec{"aod-metadata-source", VariantType::String, filename, {"File from which the metadata was extracted."}}); + plainKeys = metadataKeys(top.metadata); + } else if (parentAccessLevel == 0) { LOGP(info, "No metadata found in file \"{}\" and parent level 0 prevents further lookup.", filename); results.push_back(ConfigParamSpec{"aod-metadata-disable", VariantType::String, "1", {"Metadata not found in AOD"}}); - return results; + } else { + // Lets try in parent file. + LOGP(info, "No metadata found in file \"{}\", checking in its parents.", filename); + auto const& parent = readLevel(1); + if (!parent.found) { + LOGP(info, "No metadata found in file \"{}\"", filename); + } else { + results = metadataToSpecs(parent.metadata); + if (!parent.metadata.empty()) { + if (!parent.tables.empty()) { + results.push_back(ConfigParamSpec{"aod-metadata-tables", VariantType::ArrayString, parent.tables, {"Tables in first AOD"}}); + } + results.push_back(ConfigParamSpec{"aod-metadata-source", VariantType::String, filename, {"File from which the metadata was extracted."}}); + plainKeys = metadataKeys(parent.metadata); + } else { + LOGP(info, "No metadata found in file \"{}\" nor in its parent file \"{}\"", filename, parent.source); + } + } + if (plainKeys.empty()) { + results.push_back(ConfigParamSpec{"aod-metadata-disable", VariantType::String, "1", {"Metadata not found in AOD"}}); + } } - // Lets try in parent file. - auto parentFiles = (TMap*)currentFile->Get("parentFiles"); - if (!parentFiles) { - LOGP(info, "No metadata found in file \"{}\"", filename); - results.push_back(ConfigParamSpec{"aod-metadata-disable", VariantType::String, "1", {"Metadata not found in AOD"}}); + if (!registry.hasOption("aod-origin-level-map")) { return results; } - LOGP(info, "No metadata found in file \"{}\", checking in its parents.", filename); - for (auto* p : *parentFiles) { - std::string parentFilename = ((TPair*)p)->Value()->GetName(); - // Do the replacement. Notice this will require changing aod-parent-base-path-replacement to be - // a workflow option (because the metadata itself is potentially changing the topology). - if (registry.isSet("aod-parent-base-path-replacement")) { - auto parentFileReplacement = registry.get("aod-parent-base-path-replacement"); - auto pos = parentFileReplacement.find(';'); - if (pos == std::string::npos) { - throw std::runtime_error(fmt::format("Invalid syntax in aod-parent-base-path-replacement: \"{}\"", parentFileReplacement.c_str())); - } - auto from = parentFileReplacement.substr(0, pos); - auto to = parentFileReplacement.substr(pos + 1); - pos = parentFilename.find(from); - if (pos != std::string::npos) { - parentFilename.replace(pos, from.length(), to); + + // The metadata of the level each origin reads, one row per origin. The columns are + // all the keys found, including the plain ones, so that an empty cell always means + // that the key is missing for that origin. + auto levelMap = registry.get>("aod-origin-level-map"); + auto origins = levelMap.getLabelsRows(); + std::vector keys = plainKeys; + for (auto i = 0U; i < levelMap.rows(); ++i) { + auto level = levelMap.get(i, 0U); + if (level > parentAccessLevel) { + LOGP(fatal, "Origin {} is mapped to parent level {}, but aod-parent-access-level is {}", origins[i], level, parentAccessLevel); + } + for (auto const& [key, value] : readLevel(level).metadata) { + if (std::ranges::find(keys, key) == keys.end()) { + keys.push_back(key); } } + } + auto sourceColumn = keys.size(); + keys.emplace_back("source"); + auto disableColumn = keys.size(); + keys.emplace_back("disable"); - if (parentFilename.starts_with("alien://") && !gGrid) { - TGrid::Connect("alien://"); + std::vector values(origins.size() * keys.size()); + for (auto i = 0U; i < origins.size(); ++i) { + auto const& info = readLevel(levelMap.get(i, 0U)); + auto* row = values.data() + i * keys.size(); + for (auto const& [key, value] : info.metadata) { + row[std::ranges::find(keys, key) - keys.begin()] = value; } - - std::unique_ptr parentFile{TFile::Open(parentFilename.c_str())}; - if (parentFile.get() == nullptr) { - LOGP(fatal, "Couldn't open derived file \"{}\"!", parentFilename); + row[sourceColumn] = info.source; + if (info.metadata.empty()) { + LOGP(info, "Metadata info missing for origin {}", origins[i]); + row[disableColumn] = "1"; } - results = readMetadata(parentFile); - // Found metadata already in the main file. - if (!results.empty()) { - auto tables = getListOfTables(parentFile); - if (tables.empty() == false) { - results.push_back(ConfigParamSpec{"aod-metadata-tables", VariantType::ArrayString, tables, {"Tables in first AOD"}}); - } - results.push_back(ConfigParamSpec{"aod-metadata-source", VariantType::String, filename, {"File from which the metadata was extracted."}}); - return results; + } + LabeledArray originMetadata{values.data(), static_cast(origins.size()), static_cast(keys.size()), origins, keys}; + results.push_back(ConfigParamSpec{"aod-metadata-origins", VariantType::LabeledArrayString, originMetadata, {"Metadata in AOD for each origin: one row per origin, one column per key"}}); + for (auto i = 0U; i < origins.size(); ++i) { + auto const& tables = readLevel(levelMap.get(i, 0U)).tables; + if (!tables.empty()) { + results.push_back(ConfigParamSpec{"aod-metadata-origin-" + origins[i] + "-tables", VariantType::ArrayString, tables, {"Tables in the AOD read for this origin"}}); } - LOGP(info, "No metadata found in file \"{}\" nor in its parent file \"{}\"", filename, parentFilename); - break; } - results.push_back(ConfigParamSpec{"aod-metadata-disable", VariantType::String, "1", {"Metadata not found in AOD"}}); return results; }}; } diff --git a/Framework/Core/include/Framework/Array2D.h b/Framework/Core/include/Framework/Array2D.h index 857e4b3c89f29..4b21ac2ba5b32 100644 --- a/Framework/Core/include/Framework/Array2D.h +++ b/Framework/Core/include/Framework/Array2D.h @@ -79,19 +79,29 @@ struct Array2D { Array2D& operator=(Array2D const& other) { - this->rows = other.rows; - this->cols = other.cols; - data = new T[rows * cols]; - for (auto i = 0U; i < rows; ++i) { - for (auto j = 0U; j < cols; ++j) { - data[i * cols + j] = *(other.data + (i * cols + j)); + if (this == &other) { + return *this; + } + // Copy into a new buffer first, so that a throwing copy leaves this array untouched + auto* newData = new T[other.rows * other.cols]; + for (auto i = 0U; i < other.rows; ++i) { + for (auto j = 0U; j < other.cols; ++j) { + newData[i * other.cols + j] = other.data[i * other.cols + j]; } } + delete[] data; + data = newData; + this->rows = other.rows; + this->cols = other.cols; return *this; } Array2D& operator=(Array2D&& other) { + if (this == &other) { + return *this; + } + delete[] data; this->rows = other.rows; this->cols = other.cols; data = other.data; @@ -231,6 +241,13 @@ class LabeledArray : public LabelMap return values[y]; } + // nullptr if there is no row with this label + T* getRow(const char* y) const + { + auto it = rowmap.find(y); + return it == rowmap.end() ? nullptr : values[it->second]; + } + T* operator[](uint32_t y) const { return values[y]; diff --git a/Framework/Core/include/Framework/ConfigParamDiscovery.h b/Framework/Core/include/Framework/ConfigParamDiscovery.h index 3d13d3662ceb1..c64c4c8255cb9 100644 --- a/Framework/Core/include/Framework/ConfigParamDiscovery.h +++ b/Framework/Core/include/Framework/ConfigParamDiscovery.h @@ -13,6 +13,9 @@ #define O2_FRAMEWORK_CONFIGPARAMDISCOVERY_H_ #include "Framework/ConfigParamRegistry.h" +#include "Framework/Array2D.h" + +#include namespace o2::framework { @@ -20,6 +23,16 @@ namespace o2::framework // options. struct ConfigParamDiscovery { static std::vector discover(ConfigParamRegistry&, int, char**); + + /// Parse the value of aod-origin-level-mapping, "ORIGIN:LEVEL[,ORIGIN2:LEVEL2,...]", + /// into a LabeledArray with one row per origin and a single "level" column. + /// Throws std::runtime_error if the mapping is malformed. + static LabeledArray parseOriginLevelMapping(std::string_view mapping); + + /// Replace the plain aod-metadata-* options with the ones in the row of + /// aod-metadata-origins which corresponds to the origin this process reads + /// (aod-origin-replace, or AOD). Does nothing if there is no such row. + static void applyOriginMetadata(ConfigParamRegistry& registry); }; struct ConfigDiscovery { 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/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/Variant.h b/Framework/Core/include/Framework/Variant.h index 7121c5fad0669..3611fad438e0a 100644 --- a/Framework/Core/include/Framework/Variant.h +++ b/Framework/Core/include/Framework/Variant.h @@ -13,6 +13,7 @@ #include "Framework/RuntimeError.h" #include "Framework/Array2D.h" +#include "Framework/Traits.h" #include #include #include @@ -261,9 +262,9 @@ struct variant_helper { new (reinterpret_cast(store)) T{}; *(reinterpret_cast(store)) = value; } - static void set(void* store, T values, size_t size) + static void set(void* store, std::remove_pointer_t const* values, size_t size) { - *reinterpret_cast(store) = reinterpret_cast(std::memcpy(std::malloc(size * sizeof(std::remove_pointer_t)), reinterpret_cast(values), size * sizeof(std::remove_pointer_t))); + *reinterpret_cast(store) = reinterpret_cast(std::memcpy(std::malloc(size * sizeof(std::remove_pointer_t)), reinterpret_cast(values), size * sizeof(std::remove_pointer_t))); } static T get(const void* store) { return *(reinterpret_cast(store)); } @@ -317,9 +318,10 @@ struct variant_helper { class Variant { public: - Variant(VariantType type = VariantType::Unknown) : mType{type} {} + Variant(VariantType type = VariantType::Unknown); template + requires(!is_specialization_v) Variant(T value) : mType{variant_trait_v} { variant_helper::set(&mStore, value); @@ -331,17 +333,23 @@ class Variant variant_helper::set(&mStore, values, mSize); } + // A Variant owns what it stores: the content of vectors is copied template - Variant(std::vector& values) : mType{variant_trait_v}, mSize{values.size()} + Variant(std::vector const& values) : mType{variant_trait_v}, mSize{values.size()} { variant_helper::set(&mStore, values.data(), mSize); } - Variant(std::vector& values) : mType{VariantType::ArrayString}, mSize{values.size()} + Variant(std::vector const& values) : mType{VariantType::ArrayString}, mSize{values.size()} { variant_helper>::set(&mStore, values); } + // A temporary vector cannot hand over its buffer to a Variant, + // so creating one only to copy it is an error + template + Variant(std::vector&&) = delete; + template Variant(std::initializer_list) { @@ -355,6 +363,8 @@ class Variant ~Variant(); Variant& operator=(const Variant& other); Variant& operator=(Variant&& other) noexcept; + // Assignment from a temporary vector has to be allowed because it is used, but copies + // to make sure Variant owns its content template Variant& operator=(std::vector&& other) noexcept { @@ -381,31 +391,25 @@ class Variant return variant_helper::get(&mStore); } + // The setters replace the current content, releasing it first. template void set(T value) { - return variant_helper::set(&mStore, value); + *this = Variant(value); } template void set(T value, size_t size) { - mSize = size; - return variant_helper::set(&mStore, value, mSize); - } - - template - void set(std::vector& values) - requires(std::is_pod_v) - { - return variant_helper::set(&mStore, values.data(), values.size()); + *this = Variant(value, size); } + /// FIXME: set for vector of strings is not used anywhere, why? template void set(std::vector& values) - requires(std::is_same_v) + requires(std::is_pod_v || std::is_same_v) { - return variant_helper::set(&mStore, values); + *this = Variant(values); } [[nodiscard]] VariantType type() const { return mType; } @@ -414,6 +418,10 @@ class Variant private: friend std::ostream& operator<<(std::ostream& oss, Variant const& val); + // Helpers to manage the store depending on the actual content + void copyStore(Variant const& other); + void moveStore(Variant& other) noexcept; + void destroyStore() noexcept; using storage_t = std::aligned_union<8, int, int8_t, int16_t, int64_t, uint8_t, uint16_t, uint32_t, uint64_t, const char*, float, double, bool, 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/ConfigParamDiscovery.cxx b/Framework/Core/src/ConfigParamDiscovery.cxx index fc8d6f2600bb4..76448ea8222db 100644 --- a/Framework/Core/src/ConfigParamDiscovery.cxx +++ b/Framework/Core/src/ConfigParamDiscovery.cxx @@ -16,6 +16,12 @@ #include #include #include +#include + +#include +#include +#include +#include O2_DECLARE_DYNAMIC_LOG(config_parameter_discovery); @@ -77,7 +83,90 @@ std::vector ConfigParamDiscovery::discover(ConfigParamRegistry& } registry.loadExtra(extras); } + applyOriginMetadata(registry); return result; } +LabeledArray ConfigParamDiscovery::parseOriginLevelMapping(std::string_view mapping) +{ + std::vector origins; + std::vector levels; + for (auto pairRange : mapping | std::views::split(',')) { + std::string_view pair{pairRange.begin(), pairRange.end()}; + auto colonPos = pair.find(':'); + if (colonPos == std::string_view::npos) { + throw std::runtime_error(fmt::format("Badly formatted aod-origin-level-mapping entry: \"{}\"", pair)); + } + std::string origin{pair.substr(0, colonPos)}; + if (origin.empty() || origin.size() > 4) { + throw std::runtime_error(fmt::format("Origin in aod-origin-level-mapping entry must have 1 to 4 characters: \"{}\"", pair)); + } + if (std::ranges::find(origins, origin) != origins.end()) { + throw std::runtime_error(fmt::format("Duplicate origin in aod-origin-level-mapping: \"{}\"", origin)); + } + auto levelStr = pair.substr(colonPos + 1); + int level{}; + auto [ptr, ec] = std::from_chars(levelStr.data(), levelStr.data() + levelStr.size(), level); + if (levelStr.empty() || ec != std::errc{} || ptr != levelStr.data() + levelStr.size() || level < 0) { + throw std::runtime_error(fmt::format("Unable to parse level in aod-origin-level-mapping entry: \"{}\"", pair)); + } + origins.push_back(std::move(origin)); + levels.push_back(level); + } + if (origins.empty()) { + throw std::runtime_error("Empty aod-origin-level-mapping"); + } + return LabeledArray{levels.data(), static_cast(levels.size()), 1, origins, {"level"}}; +} + +void ConfigParamDiscovery::applyOriginMetadata(ConfigParamRegistry& registry) +{ + if (!registry.hasOption("aod-metadata-origins")) { + return; + } + std::string origin = "AOD"; + if (registry.hasOption("aod-origin-replace")) { + auto replace = registry.get("aod-origin-replace"); + if (!replace.empty()) { + origin = replace; + } + } + auto metadata = registry.get>("aod-metadata-origins"); + auto* row = metadata.getRow(origin.c_str()); + if (row == nullptr) { + return; + } + O2_SIGNPOST_ID_GENERATE(sid, config_parameter_discovery); + auto const& keys = metadata.getLabelsCols(); + auto disable = metadata.colmap.find("disable"); + if (disable != metadata.colmap.end() && row[disable->second] == "1") { + O2_SIGNPOST_EVENT_EMIT(config_parameter_discovery, sid, "origin metadata", "No metadata for origin %{public}s", origin.c_str()); + for (auto const& key : keys) { + registry.erase(("aod-metadata-" + key).c_str()); + } + registry.erase("aod-metadata-tables"); + registry.override("aod-metadata-disable", std::string{"1"}); + return; + } + O2_SIGNPOST_EVENT_EMIT(config_parameter_discovery, sid, "origin metadata", "Using metadata for origin %{public}s", origin.c_str()); + for (auto j = 0U; j < metadata.cols(); ++j) { + if (keys[j] == "disable") { + continue; + } + auto key = "aod-metadata-" + keys[j]; + if (row[j].empty()) { + registry.erase(key.c_str()); + } else { + registry.override(key.c_str(), row[j]); + } + } + registry.erase("aod-metadata-disable"); + auto tablesKey = "aod-metadata-origin-" + origin + "-tables"; + if (registry.hasOption(tablesKey.c_str())) { + registry.override("aod-metadata-tables", registry.get>(tablesKey.c_str())); + } else { + registry.erase("aod-metadata-tables"); + } +} + } // namespace o2::framework diff --git a/Framework/Core/src/ConfigParamRegistry.cxx b/Framework/Core/src/ConfigParamRegistry.cxx index 0e895bc38e063..4521bd693fe57 100644 --- a/Framework/Core/src/ConfigParamRegistry.cxx +++ b/Framework/Core/src/ConfigParamRegistry.cxx @@ -12,6 +12,9 @@ #include "Framework/VariantPropertyTreeHelpers.h" #include "Framework/Array2D.h" +#include +#include + namespace o2::framework { @@ -67,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 @@ -83,8 +99,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 (...) { @@ -92,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) { @@ -143,16 +176,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/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/Plugin.cxx b/Framework/Core/src/Plugin.cxx index d1abb44c3cc80..ffe4ed34d40ff 100644 --- a/Framework/Core/src/Plugin.cxx +++ b/Framework/Core/src/Plugin.cxx @@ -129,10 +129,14 @@ struct DiscoverMetadataInCommandLine : o2::framework::ConfigDiscoveryPlugin { std::string value = argv[i + 1]; O2_SIGNPOST_EVENT_EMIT(capabilities, sid, "DiscoverMetadataInCommandLine", "Found %{public}s with value %{public}s.", key.c_str(), value.c_str()); - if (key == "aod-metadata-tables") { + if (key == "aod-metadata-tables" || (key.starts_with("aod-metadata-origin-") && key.ends_with("-tables"))) { std::stringstream is(value); auto arrayValue = VariantJSONHelpers::read(is); results.push_back(ConfigParamSpec{key, VariantType::ArrayString, arrayValue, {"Metadata in command line"}}); + } else if (key == "aod-metadata-origins") { + std::stringstream is(value); + auto arrayValue = VariantJSONHelpers::read(is); + results.push_back(ConfigParamSpec{key, VariantType::LabeledArrayString, arrayValue, {"Metadata in command line"}}); } else { results.push_back(ConfigParamSpec{key, VariantType::String, value, {"Metadata in command line"}}); } @@ -175,6 +179,13 @@ struct DiscoverAODOptionsInCommandLine : o2::framework::ConfigDiscoveryPlugin { } if (key == "aod-origin-level-mapping") { results.push_back(ConfigParamSpec{"aod-origin-level-mapping", VariantType::String, value, {"Map origin to parent level for AOD reading. Syntax: ORIGIN:LEVEL[,ORIGIN2:LEVEL2,...]. E.g. \"DYN:1\"."}}); + LabeledArray levelMap; + try { + levelMap = ConfigParamDiscovery::parseOriginLevelMapping(value); + } catch (std::runtime_error const& e) { + LOGP(fatal, "{}", e.what()); + } + results.push_back(ConfigParamSpec{"aod-origin-level-map", VariantType::LabeledArrayInt, levelMap, {"Parsed aod-origin-level-mapping: one row per origin, with its parent level"}}); } } if (injectOption) { 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 e54a973bd4413..b982ffd22c99e 100644 --- a/Framework/Core/src/Variant.cxx +++ b/Framework/Core/src/Variant.cxx @@ -11,7 +11,10 @@ #include "Framework/Variant.h" #include "Framework/VariantPropertyTreeHelpers.h" #include "Framework/VariantJSONHelpers.h" +#include +#include #include +#include #include namespace o2::framework @@ -63,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: @@ -84,145 +91,166 @@ std::string Variant::asString() const return ss.str(); } -Variant::Variant(const Variant& other) : mType(other.mType) +namespace { - // In case this is an array we need to duplicate it to avoid - // double deletion. - switch (mType) { - case VariantType::String: - mSize = other.mSize; - variant_helper::set(&mStore, other.get()); - return; - case VariantType::ArrayInt: - mSize = other.mSize; - variant_helper::set(&mStore, other.get(), mSize); - return; - case VariantType::ArrayFloat: - mSize = other.mSize; - variant_helper::set(&mStore, other.get(), mSize); - return; - case VariantType::ArrayDouble: - mSize = other.mSize; - variant_helper::set(&mStore, other.get(), mSize); - return; - case VariantType::ArrayBool: - mSize = other.mSize; - variant_helper::set(&mStore, other.get(), mSize); - return; +/// Helper visitor for Variant +template +bool visitStoredObject(VariantType type, F&& f) +{ + switch (type) { case VariantType::ArrayString: - mSize = other.mSize; - variant_helper>::set(&mStore, other.get>()); - return; + f.template operator()>(); + return true; + case VariantType::Array2DInt: + f.template operator()>(); + return true; + case VariantType::Array2DFloat: + f.template operator()>(); + return true; + case VariantType::Array2DDouble: + f.template operator()>(); + return true; + case VariantType::LabeledArrayInt: + f.template operator()>(); + return true; + case VariantType::LabeledArrayFloat: + f.template operator()>(); + return true; + case VariantType::LabeledArrayDouble: + f.template operator()>(); + return true; + case VariantType::LabeledArrayString: + f.template operator()>(); + return true; default: - mStore = other.mStore; - mSize = other.mSize; + return false; } } -Variant::Variant(Variant&& other) noexcept : mType(other.mType) +/// Types for which the storage keeps a pointer to a manually allocated buffer +bool holdsMallocedPointer(VariantType type) { - mStore = other.mStore; - mSize = other.mSize; - switch (mType) { + switch (type) { case VariantType::String: case VariantType::ArrayInt: case VariantType::ArrayFloat: case VariantType::ArrayDouble: case VariantType::ArrayBool: - case VariantType::ArrayString: - *reinterpret_cast(&(other.mStore)) = nullptr; - return; + return true; default: - return; + return false; } } -Variant::~Variant() +template +T* copyBuffer(T const* values, size_t size) +{ + if (values == nullptr) { + return nullptr; + } + return reinterpret_cast(std::memcpy(std::malloc(size * sizeof(T)), values, size * sizeof(T))); +} +} // namespace + +Variant::Variant(VariantType type) : mType{type} +{ + // Make sure that destroying a Variant created without a value is always safe + // by creating a default stored object upfront + if (!visitStoredObject(mType, [this]() { new (&mStore) T{}; })) { + std::memset(&mStore, 0, sizeof(mStore)); + } +} + +void Variant::copyStore(Variant const& other) { - // In case we allocated an array, we - // should delete it. + // Proper objects are simply copied + if (visitStoredObject(mType, [this, &other]() { new (&mStore) T(*reinterpret_cast(&other.mStore)); })) { + return; + } + // Manually allocated buffers have to be managed switch (mType) { - case VariantType::String: + case VariantType::String: { + auto const* value = *reinterpret_cast(&other.mStore); + *reinterpret_cast(&mStore) = value != nullptr ? strdup(value) : nullptr; + return; + } case VariantType::ArrayInt: + *reinterpret_cast(&mStore) = copyBuffer(*reinterpret_cast(&other.mStore), mSize); + return; case VariantType::ArrayFloat: + *reinterpret_cast(&mStore) = copyBuffer(*reinterpret_cast(&other.mStore), mSize); + return; case VariantType::ArrayDouble: - case VariantType::ArrayBool: { - if (reinterpret_cast(&mStore) != nullptr) { - free(*reinterpret_cast(&mStore)); - } + *reinterpret_cast(&mStore) = copyBuffer(*reinterpret_cast(&other.mStore), mSize); return; - } - case VariantType::ArrayString: { - // Allocated with placement new. Nothing to delete. + case VariantType::ArrayBool: + *reinterpret_cast(&mStore) = copyBuffer(*reinterpret_cast(&other.mStore), mSize); return; - } default: - return; + // Trivially copyable content + mStore = other.mStore; + } +} + +void Variant::moveStore(Variant& other) noexcept +{ + // Correct move for objects, leaving proper "moved from" state + if (visitStoredObject(mType, [this, &other]() { new (&mStore) T(std::move(*reinterpret_cast(&other.mStore))); })) { + return; + } + mStore = other.mStore; + // Buffers have to change their owner + if (holdsMallocedPointer(mType)) { + *reinterpret_cast(&other.mStore) = nullptr; + } +} + +void Variant::destroyStore() noexcept +{ + // destroy objects + if (visitStoredObject(mType, [this]() { std::destroy_at(reinterpret_cast(&mStore)); })) { + return; + } + // deallocate buffers + if (holdsMallocedPointer(mType)) { + free(*reinterpret_cast(&mStore)); } } +Variant::Variant(const Variant& other) : mType(other.mType), mSize(other.mSize) +{ + copyStore(other); +} + +Variant::Variant(Variant&& other) noexcept : mType(other.mType), mSize(other.mSize) +{ + moveStore(other); +} + +Variant::~Variant() +{ + destroyStore(); +} + Variant& Variant::operator=(const Variant& other) { - mSize = other.mSize; - mType = other.mType; - switch (mType) { - case VariantType::String: - variant_helper::set(&mStore, other.get()); - return *this; - case VariantType::ArrayInt: - variant_helper::set(&mStore, other.get(), mSize); - return *this; - case VariantType::ArrayFloat: - variant_helper::set(&mStore, other.get(), mSize); - return *this; - case VariantType::ArrayDouble: - variant_helper::set(&mStore, other.get(), mSize); - return *this; - case VariantType::ArrayBool: - variant_helper::set(&mStore, other.get(), mSize); - return *this; - case VariantType::ArrayString: - variant_helper>::set(&mStore, other.get>()); - return *this; - default: - mStore = other.mStore; - return *this; + if (this != &other) { + // Copy first, so that a throwing copy leaves this Variant untouched + Variant copy(other); + *this = std::move(copy); } + return *this; } Variant& Variant::operator=(Variant&& other) noexcept { - mSize = other.mSize; - mType = other.mType; - switch (mType) { - case VariantType::String: - variant_helper::set(&mStore, other.get()); - *reinterpret_cast(&(other.mStore)) = nullptr; - return *this; - case VariantType::ArrayInt: - variant_helper::set(&mStore, other.get(), mSize); - *reinterpret_cast(&(other.mStore)) = nullptr; - return *this; - case VariantType::ArrayFloat: - variant_helper::set(&mStore, other.get(), mSize); - *reinterpret_cast(&(other.mStore)) = nullptr; - return *this; - case VariantType::ArrayDouble: - variant_helper::set(&mStore, other.get(), mSize); - *reinterpret_cast(&(other.mStore)) = nullptr; - return *this; - case VariantType::ArrayBool: - variant_helper::set(&mStore, other.get(), mSize); - *reinterpret_cast(&(other.mStore)) = nullptr; - return *this; - case VariantType::ArrayString: - variant_helper>::set(&mStore, other.get>()); - *reinterpret_cast**>(&(other.mStore)) = nullptr; - return *this; - default: - mStore = other.mStore; - return *this; + if (this != &other) { + destroyStore(); + mType = other.mType; + mSize = other.mSize; + moveStore(other); } + return *this; } std::pair, std::vector> extractLabels(boost::property_tree::ptree const& tree) 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_ConfigParamRegistry.cxx b/Framework/Core/test/test_ConfigParamRegistry.cxx index ed3dd95ad2786..346ebdb1b3e8d 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,139 @@ 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"}); +} + +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}); +} diff --git a/Framework/Core/test/test_DeviceSpecHelpers.cxx b/Framework/Core/test/test_DeviceSpecHelpers.cxx index 4184be848c5ec..bf163f919c795 100644 --- a/Framework/Core/test/test_DeviceSpecHelpers.cxx +++ b/Framework/Core/test/test_DeviceSpecHelpers.cxx @@ -101,12 +101,19 @@ void check(const std::vector& arguments, TEST_CASE("test_prepareArguments") { + int lm[1][2] = {{1, 2}}; + std::string om[2][3] = {{"MC", "/path/L0.root", ""}, {"", "/path/L1.root", "1"}}; 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"}}, + {"aod-metadata-origins", VariantType::LabeledArrayString, LabeledArray{&om[0][0], 2, 3, {"EMB", "AOD"}, {"DataType", "source", "disable"}}, {"metadata per origin"}}, }; + std::string labeledDefault = R"({"labels_rows":["r"],"labels_cols":["c1","c2"],"values":[[1,2]]})"; + std::string originsDefault = R"({"labels_rows":["EMB","AOD"],"labels_cols":["DataType","source","disable"],"values":[["MC","/path/L0.root",""],["","/path/L1.root","1"]]})"; + 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 +184,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}, {"--aod-metadata-origins", originsDefault}}; + 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 da1f39f241e96..bbdb0fff45e52 100644 --- a/Framework/Core/test/test_Variants.cxx +++ b/Framework/Core/test/test_Variants.cxx @@ -229,6 +229,146 @@ TEST_CASE("Array2DTest") } } +TEST_CASE("Array2DAssignmentTest") +{ + std::vector v = {1, 2, 3, 4, 5, 6}; + Array2D source(v, 2, 3); + + // copy assignment to an array of different shape makes an independent deep copy + Array2D copied(std::vector{7}, 1, 1); + copied = source; + REQUIRE(copied.rows == 2); + REQUIRE(copied.cols == 3); + REQUIRE(copied.data != source.data); + source[0][0] = 42; + REQUIRE(copied(0, 0) == 1); + for (auto i = 1U; i < 6; ++i) { + REQUIRE(copied(i / 3, i % 3) == v[i]); + } + + // self copy assignment keeps the contents + auto& self = copied; + copied = self; + REQUIRE(copied.rows == 2); + REQUIRE(copied.cols == 3); + REQUIRE(copied(1, 2) == 6); + + // move assignment takes over the buffer and empties the source + auto* buffer = copied.data; + Array2D moved(std::vector{7, 8}, 1, 2); + moved = std::move(copied); + REQUIRE(moved.data == buffer); + REQUIRE(moved.rows == 2); + REQUIRE(moved.cols == 3); + REQUIRE(moved(1, 2) == 6); + REQUIRE(copied.data == nullptr); + REQUIRE(copied.rows == 0); + REQUIRE(copied.cols == 0); + + // self move assignment keeps the contents + auto& selfMoved = moved; + moved = std::move(selfMoved); + REQUIRE(moved.data == buffer); + REQUIRE(moved(1, 2) == 6); + + // assigning an empty array releases the old buffer and leaves an empty array + moved = Array2D{}; + REQUIRE(moved.data == nullptr); + REQUIRE(moved.rows == 0); + REQUIRE(moved.cols == 0); + + // strings are copied element by element + std::vector s = {"one", "two"}; + Array2D ms(s, 2, 1); + Array2D msc; + msc = ms; + ms[0][0] = "changed"; + REQUIRE(msc(0, 0) == "one"); + REQUIRE(msc(1, 0) == "two"); +} + +TEST_CASE("VariantLifecycleTest") +{ + float m[2][3] = {{1, 2, 3}, {4, 5, 6}}; + LabeledArray laf{&m[0][0], 2, 3, {"r1", "r2"}, {"c1", "c2", "c3"}}; + std::vector vs{"s1", "s2", "s3"}; + auto checkLabeled = [&](Variant const& v) { + REQUIRE(v.type() == VariantType::LabeledArrayFloat); + auto la = v.get>(); + REQUIRE(la.rows() == 2); + REQUIRE(la.cols() == 3); + REQUIRE(la.get("r2", "c3") == 6); + REQUIRE(la.getLabelsRows() == std::vector{"r1", "r2"}); + }; + + Variant vl(laf); + // copies are independent and survive the destruction of the original + auto* copy = new Variant(vl); + Variant moved(std::move(*copy)); + delete copy; + checkLabeled(vl); + checkLabeled(moved); + + // assignment across types releases the previous content + Variant vstr("a string"); + vstr = vl; + checkLabeled(vstr); + vstr = Variant("back to a string"); + REQUIRE(vstr.type() == VariantType::String); + REQUIRE(std::string(vstr.get()) == "back to a string"); + vstr = std::move(moved); + checkLabeled(vstr); + Variant vvs(vs); + vstr = vvs; + REQUIRE(vstr.get>() == vs); + + // self assignment keeps the content + auto& self = vl; + vl = self; + checkLabeled(vl); + vl = std::move(self); + checkLabeled(vl); + + // moving a string array leaves a valid moved-from Variant behind + Variant vvsMoved(std::move(vvs)); + REQUIRE(vvsMoved.get>() == vs); + vvs = vvsMoved; + REQUIRE(vvs.get>() == vs); + + // reallocation of a container moves the Variants around + std::vector collection; + for (auto i = 0; i < 20; ++i) { + collection.emplace_back(laf); + collection.emplace_back(Array2D{&m[0][0], 2, 3}); + collection.emplace_back(vs); + collection.emplace_back("a string"); + } + std::vector collectionCopy = collection; + collection.clear(); + for (auto i = 0U; i < collectionCopy.size(); i += 4) { + checkLabeled(collectionCopy[i]); + REQUIRE(collectionCopy[i + 1].get>()(1, 2) == 6); + REQUIRE(collectionCopy[i + 2].get>() == vs); + REQUIRE(std::string(collectionCopy[i + 3].get()) == "a string"); + } + + // a Variant created with only a type can be copied and destroyed + Variant typed(VariantType::LabeledArrayFloat); + Variant typedCopy(typed); + REQUIRE(typedCopy.get>().rows() == 0); + Variant typedString(VariantType::String); + Variant typedStringCopy(typedString); + REQUIRE(typedStringCopy.get() == nullptr); + + // set replaces the content and the type + Variant vset(1); + vset.set(laf); + checkLabeled(vset); + vset.set(vs); + REQUIRE(vset.type() == VariantType::ArrayString); + REQUIRE(vset.get>() == vs); +} + TEST_CASE("LabeledArrayTest") { float m[3][4] = {{0.1, 0.2, 0.3, 0.4}, {0.5, 0.6, 0.7, 0.8}, {0.9, 1.0, 1.1, 1.2}}; @@ -247,17 +387,24 @@ TEST_CASE("LabeledArrayTest") REQUIRE(las.get(i, xl[j].c_str()) == las.get(i, j)); REQUIRE(las.get(yl[i].c_str(), j) == las.get(i, j)); } + REQUIRE(laf.getRow(yl[i].c_str()) == laf.getRow(i)); + REQUIRE(las.getRow(yl[i].c_str()) == las.getRow(i)); } + REQUIRE(laf.getRow("r4") == nullptr); + REQUIRE(las.getRow("r4") == nullptr); } TEST_CASE("VariantTreeConversionsTest") { std::vector vstrings{"0 1", "0 2", "0 3"}; - Variant vvstr(std::move(vstrings)); + Variant vvstr(vstrings); auto tree = vectorToBranch(vvstr.get(), vvstr.size()); - auto v = Variant(vectorFromBranch(tree)); + auto fromTree = vectorFromBranch(tree); + auto v = Variant(fromTree); + REQUIRE(vvstr.size() == vstrings.size()); + REQUIRE(v.size() == vstrings.size()); for (auto i = 0U; i < vvstr.size(); ++i) { REQUIRE(vvstr.get()[i] == v.get()[i]); } @@ -352,3 +499,126 @@ TEST_CASE("VariantThrowing") REQUIRE(error.what == std::string("Variant::get: Mismatch between types 4 0.")); } } + +// A Variant can be created from a vector only by copying its content +static_assert(!std::is_constructible_v>); +static_assert(!std::is_constructible_v>); +static_assert(!std::is_constructible_v>); +static_assert(!std::is_constructible_v>); +static_assert(!std::is_constructible_v>); +// assignment from a temporary vector copies it +static_assert(std::is_assignable_v>); +static_assert(std::is_assignable_v>); +static_assert(std::is_constructible_v&>); +static_assert(std::is_constructible_v const&>); +static_assert(std::is_constructible_v const&>); +static_assert(std::is_assignable_v const&>); + +namespace +{ +template +void checkVectorCopy(VariantType type, std::vector source) +{ + auto check = [&](Variant const& v, std::vector const& from) { + REQUIRE(v.type() == type); + REQUIRE(v.size() == from.size()); + auto const* stored = v.get(); + REQUIRE(stored != from.data()); + for (auto i = 0U; i < from.size(); ++i) { + REQUIRE(stored[i] == from[i]); + } + }; + std::vector const constSource = source; + Variant fromConst(constSource); + check(fromConst, constSource); + + Variant fromMutable(source); + check(fromMutable, source); + // the Variant owns a copy, unaffected by later changes of the source + auto const original = source; + source[0] = source.back(); + source.push_back(source[0]); + check(fromMutable, original); +} +} // namespace + +TEST_CASE("VariantFromVectorTest") +{ + checkVectorCopy(VariantType::ArrayInt, {1, 2, 3, 4}); + checkVectorCopy(VariantType::ArrayFloat, {0.5f, 1.5f, 2.5f}); + checkVectorCopy(VariantType::ArrayDouble, {1e-3, 1e3}); + checkVectorCopy(VariantType::ArrayString, {"a", "bb", "ccc"}); + + // assignment from a vector copies as well + std::vector vi{7, 8, 9}; + Variant v(1); + v = vi; + REQUIRE(v.type() == VariantType::ArrayInt); + REQUIRE(v.size() == 3); + REQUIRE(v.get() != vi.data()); + REQUIRE(v.get()[2] == 9); + // also from a temporary + auto makeStrings = []() { return std::vector{"x", "y"}; }; + v = makeStrings(); + REQUIRE(v.type() == VariantType::ArrayString); + REQUIRE(v.size() == 2); + REQUIRE(v.get()[1] == "y"); + v = std::vector{1.5, 2.5, 3.5}; + REQUIRE(v.type() == VariantType::ArrayDouble); + REQUIRE(v.size() == 3); + REQUIRE(v.get()[2] == 3.5); + + // a Variant created from a const vector round-trips through JSON + std::vector const vd{0.25, 0.5, 0.75}; + Variant vdv(vd); + std::stringstream is(vdv.asString()); + auto read = VariantJSONHelpers::read(is); + REQUIRE(read.size() == vd.size()); + for (auto i = 0U; i < vd.size(); ++i) { + 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