From 6ca8805d2476a30bb8cbe307024f4ebdec786f70 Mon Sep 17 00:00:00 2001 From: Nathan Brei Date: Wed, 18 Sep 2024 13:48:40 -0400 Subject: [PATCH] Refine JWiringService --- src/libraries/JANA/Services/JWiringService.cc | 129 +++++++++++++----- src/libraries/JANA/Services/JWiringService.h | 35 +++-- .../Services/JWiringServiceTests.cc | 84 +++++++++++- 3 files changed, 191 insertions(+), 57 deletions(-) diff --git a/src/libraries/JANA/Services/JWiringService.cc b/src/libraries/JANA/Services/JWiringService.cc index ca237066d..0056e371b 100644 --- a/src/libraries/JANA/Services/JWiringService.cc +++ b/src/libraries/JANA/Services/JWiringService.cc @@ -1,8 +1,9 @@ #include "JWiringService.h" -#include "JANA/Utils/JEventLevel.h" +#include "toml.hpp" +#include #include -#include +#include namespace jana::services { @@ -11,40 +12,54 @@ void JWiringService::Init() { // User is _only_ allowed to specify wiring file via parameter // This way, we can restrict calling JWiringService::Init until inside JApplication::Init // Then we can load the wiring file exactly once. All WiredFactoryGenerators - // (Recursively) load files + // (recursively) load files + if (!m_wirings_input_file().empty()) { + AddWirings(*m_wirings_input_file); + } } -std::unique_ptr JWiringService::overlay(std::unique_ptr&& above, std::unique_ptr&& below) { +void JWiringService::AddWirings(std::vector>& wirings_bundle, const std::string& bundle_source) { - // In theory this should be handled by the caller, but let's check just in case - if (above->plugin_name != below->plugin_name) throw JException("Plugin name mismatch!"); - if (above->type_name != below->type_name) throw JException("Type name mismatch!"); + std::set prefixes_in_bundle; + for (auto& wiring: wirings_bundle) { - if (above->input_names.empty() && !below->input_names.empty()) { - above->input_names = std::move(below->input_names); - } - if (above->input_levels.empty() && !below->input_levels.empty()) { - above->input_levels = std::move(below->input_levels); - } - if (above->output_names.empty() && !below->output_names.empty()) { - above->output_names = std::move(below->output_names); - } - for (const auto& [key, val] : below->configs) { - if (above->configs.find(key) == above->configs.end()) { - above->configs[key] = val; + // Assert that this wiring's prefix is unique _within_ this bundle. + auto bundle_it = prefixes_in_bundle.find(wiring->prefix); + if (bundle_it != prefixes_in_bundle.end()) { + throw JException("Duplicated prefix '%s' in wiring bundle '%s'", wiring->prefix.c_str(), bundle_source.c_str()); + } + prefixes_in_bundle.insert(wiring->prefix); + + // Check whether we have seen this prefix before + auto it = m_wirings_from_prefix.find(wiring->prefix); + if (it == m_wirings_from_prefix.end()) { + // This is a new wiring + m_wirings_from_prefix[wiring->prefix] = wiring.get(); + m_wirings_from_type_and_plugin_names[{wiring->type_name, wiring->plugin_name}].push_back(wiring.get()); + m_wirings.push_back(std::move(wiring)); + } + else { + // Wiring is already defined; overlay this wiring _below_ the existing wiring + // First we do some sanity checks + if (wiring->type_name != it->second->type_name) { + throw JException("Wiring mismatch: type name '%s' vs '%s'", wiring->type_name.c_str(), it->second->type_name.c_str()); + } + if (wiring->plugin_name != it->second->plugin_name) { + throw JException("Wiring mismatch: plugin name '%s' vs '%s'", wiring->plugin_name.c_str(), it->second->plugin_name.c_str()); + } + Overlay(*(it->second), *wiring); + // Useful information from `wiring` has been copied into `it->second`. + // `wiring` will now be automatically destroyed } } - // below gets automatically deleted - return std::move(above); + // At this point all wirings have been moved out of wirings_bundle, so we clear it to avoid confusing callers + wirings_bundle.clear(); } - -std::vector> JWiringService::parse_table(const toml::table& table) { +void JWiringService::AddWirings(const toml::table& table, const std::string& source) { std::vector> wirings; - std::map prefix_lookup; - auto facs = table["factory"].as_array(); if (facs == nullptr) { throw JException("No factories found!"); @@ -60,16 +75,6 @@ std::vector> JWiringService::parse_table wiring->plugin_name = f["plugin_name"].value().value(); wiring->type_name = f["type_name"].value().value(); wiring->prefix = f["prefix"].value().value(); - auto it = prefix_lookup.find(wiring->prefix); - if (it != prefix_lookup.end()) { - std::ostringstream oss; - oss << "Duplicated factory prefix in wiring file: " << std::endl; - oss << " Prefix: " << wiring->prefix << std::endl; - oss << " Type name: " << wiring->type_name << " vs " << it->second->type_name << std::endl; - oss << " Plugin name: " << wiring->plugin_name << " vs " << it->second->plugin_name << std::endl; - throw JException(oss.str()); - } - prefix_lookup[wiring->prefix] = wiring.get(); wiring->level = parseEventLevel(f["level"].value_or("None")); @@ -106,9 +111,61 @@ std::vector> JWiringService::parse_table wirings.push_back(std::move(wiring)); } - return wirings; + AddWirings(wirings, source); +} + +void JWiringService::AddWirings(const std::string& filename) { + try { + auto tbl = toml::parse_file(filename); + AddWirings(tbl, filename); + } + catch (const toml::parse_error& err) { + auto e = JException("Error parsing TOML file: '%s'", filename.c_str()); + e.nested_exception = std::current_exception(); + throw e; + } } +const JWiringService::Wiring* JWiringService::GetWiring(const std::string& prefix) const { + auto it = m_wirings_from_prefix.find(prefix); + if (it == m_wirings_from_prefix.end()) { + return nullptr; + } + return it->second; +} + +const std::vector& +JWiringService::GetWirings(const std::string& plugin_name, const std::string& type_name) const { + + auto it = m_wirings_from_type_and_plugin_names.find({type_name, plugin_name}); + if (it == m_wirings_from_type_and_plugin_names.end()) { + return m_no_wirings; + } + return it->second; +} + + +void JWiringService::Overlay(Wiring& above, const Wiring& below) { + + // In theory this should be handled by the caller, but let's check just in case + if (above.plugin_name != below.plugin_name) throw JException("Plugin name mismatch!"); + if (above.type_name != below.type_name) throw JException("Type name mismatch!"); + + if (above.input_names.empty() && !below.input_names.empty()) { + above.input_names = std::move(below.input_names); + } + if (above.input_levels.empty() && !below.input_levels.empty()) { + above.input_levels = std::move(below.input_levels); + } + if (above.output_names.empty() && !below.output_names.empty()) { + above.output_names = std::move(below.output_names); + } + for (const auto& [key, val] : below.configs) { + if (above.configs.find(key) == above.configs.end()) { + above.configs[key] = val; + } + } +} diff --git a/src/libraries/JANA/Services/JWiringService.h b/src/libraries/JANA/Services/JWiringService.h index 49f313353..75e79cd72 100644 --- a/src/libraries/JANA/Services/JWiringService.h +++ b/src/libraries/JANA/Services/JWiringService.h @@ -10,8 +10,7 @@ #include #include -namespace jana { -namespace services { +namespace jana::services { class JWiringService : public JService { @@ -25,12 +24,10 @@ class JWiringService : public JService { std::vector input_names; std::vector input_levels; std::vector output_names; + std::vector output_levels; std::map configs; }; - using WiringIndex = std::map, std::map>; - // { (plugin_name,typename) : {prefix : const Wiring*}} - private: Parameter m_wirings_input_file {this, "jana:wiring_file", "wiring.toml", "Path to TOML file containing wiring definitions"}; @@ -39,18 +36,28 @@ class JWiringService : public JService { "Allow multiple definitions inside wiring files"}; std::vector> m_wirings; - WiringIndex m_wirings_index; + std::map m_wirings_from_prefix; + std::map, std::vector> m_wirings_from_type_and_plugin_names; + std::vector m_no_wirings; public: void Init() override; - void parse_file(std::string filename); - void add_wiring(std::unique_ptr wiring); - std::unique_ptr overlay(std::unique_ptr&& above, std::unique_ptr&& below); - const Wiring* get_wiring(std::string plugin_name, std::string type_name, std::string prefix); - const std::vector>& get_wirings(); - std::vector> parse_table(const toml::table& table); + void AddWirings(std::vector>& wirings, const std::string& source); + void AddWirings(const toml::table& table, const std::string& source); + void AddWirings(const std::string& filename); + + const Wiring* + GetWiring(const std::string& prefix) const; + + const std::vector& + GetWirings(const std::string& plugin_name, const std::string& type_name) const; + + const std::vector>& + GetWirings() const { return m_wirings; } + + static void Overlay(Wiring& above, const Wiring& below); }; -} // namespace services -} // namespace jana +} // namespace jana::services + diff --git a/src/programs/unit_tests/Services/JWiringServiceTests.cc b/src/programs/unit_tests/Services/JWiringServiceTests.cc index b27e3de5f..84c69494c 100644 --- a/src/programs/unit_tests/Services/JWiringServiceTests.cc +++ b/src/programs/unit_tests/Services/JWiringServiceTests.cc @@ -40,7 +40,9 @@ TEST_CASE("WiringTests") { jana::services::JWiringService sut; toml::table table = toml::parse(some_wiring); - auto wirings = sut.parse_table(table); + sut.AddWirings(table, "testcase"); + + const auto& wirings = sut.GetWirings(); REQUIRE(wirings.size() == 2); REQUIRE(wirings[0]->prefix == "myfac"); REQUIRE(wirings[1]->prefix == "myfac_modified"); @@ -74,7 +76,7 @@ TEST_CASE("WiringTests_DuplicatePrefixes") { jana::services::JWiringService sut; toml::table table = toml::parse(duplicate_prefixes); try { - auto wirings = sut.parse_table(table); + sut.AddWirings(table,"testcase"); REQUIRE(1 == 0); } catch (const JException& e) { @@ -101,11 +103,79 @@ TEST_CASE("WiringTests_Overlay") { below->configs["y"] = "42"; auto sut = jana::services::JWiringService(); - auto result = sut.overlay(std::move(above), std::move(below)); - REQUIRE(result->input_names[2] == "make"); - REQUIRE(result->input_levels[1] == JEventLevel::PhysicsEvent); - REQUIRE(result->configs["x"] == "6.18"); - REQUIRE(result->configs["y"] == "42"); + sut.Overlay(*above, *below); + REQUIRE(above->input_names[2] == "make"); + REQUIRE(above->input_levels[1] == JEventLevel::PhysicsEvent); + REQUIRE(above->configs["x"] == "6.18"); + REQUIRE(above->configs["y"] == "42"); } +static constexpr std::string_view fake_wiring_file = R"( + [[factory]] + plugin_name = "ECAL" + type_name = "ClusteringFac" + prefix = "myfac" + + [factory.configs] + x = "22" + y = "verbose" + [[factory]] + plugin_name = "ECAL" + type_name = "ClusteringFac" + prefix = "variantfac" + + [factory.configs] + x = "49" + y = "silent" + + [[factory]] + plugin_name = "BCAL" + type_name = "ClusteringFac" + prefix = "sillyfac" + + [factory.configs] + x = "618" + y = "mediocre" +)"; + + +TEST_CASE("WiringTests_FakeFacGen") { + jana::services::JWiringService sut; + toml::table table = toml::parse(fake_wiring_file); + sut.AddWirings(table, "testcase"); + + using Wiring = jana::services::JWiringService::Wiring; + std::vector> fake_facgen_wirings; + + // One gets overlaid with an existing wiring + auto a = std::make_unique(); + a->plugin_name = "ECAL"; + a->type_name = "ClusteringFac"; + a->prefix = "variantfac"; + a->configs["x"] = "42"; + fake_facgen_wirings.push_back(std::move(a)); + + // The other is brand new + auto b = std::make_unique(); + b->plugin_name = "ECAL"; + b->type_name = "ClusteringFac"; + b->prefix = "exuberantfac"; + b->configs["x"] = "27"; + fake_facgen_wirings.push_back(std::move(b)); + + // We should end up with three in total + sut.AddWirings(fake_facgen_wirings, "fake_facgen"); + auto final_wirings = sut.GetWirings("ECAL", "ClusteringFac"); + + REQUIRE(final_wirings.size() == 3); + + REQUIRE(final_wirings[0]->prefix == "myfac"); + REQUIRE(final_wirings[1]->prefix == "variantfac"); + REQUIRE(final_wirings[2]->prefix == "exuberantfac"); + + REQUIRE(final_wirings[0]->configs["x"] == "22"); // from file only + REQUIRE(final_wirings[1]->configs["x"] == "49"); // file overrides facgen + REQUIRE(final_wirings[2]->configs["x"] == "27"); // from facgen only + +}