diff --git a/phlex/configuration.cpp b/phlex/configuration.cpp index e7e08b918..65a02d0d1 100644 --- a/phlex/configuration.cpp +++ b/phlex/configuration.cpp @@ -40,12 +40,12 @@ namespace phlex { product_selector tag_invoke(boost::json::value_to_tag const&, boost::json::value const& jv) { - using detail::value_decorate_exception; + using detail::value_if_exists; auto query_object = jv.as_object(); - auto creator = value_decorate_exception(query_object, "creator"); - auto layer = value_decorate_exception(query_object, "layer"); - auto suffix = detail::value_if_exists(query_object, "suffix"); - auto stage = detail::value_if_exists(query_object, "stage"); + auto creator = value_if_exists(query_object, "creator"); + auto layer = value_if_exists(query_object, "layer"); + auto suffix = value_if_exists(query_object, "suffix"); + auto stage = value_if_exists(query_object, "stage"); return product_selector{ .creator = std::move(creator), .layer = std::move(layer), .suffix = suffix, .stage = stage}; } diff --git a/phlex/core/CMakeLists.txt b/phlex/core/CMakeLists.txt index b35c04c0f..192a9aeaa 100644 --- a/phlex/core/CMakeLists.txt +++ b/phlex/core/CMakeLists.txt @@ -1,3 +1,13 @@ +if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU") + if(CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 14.1) + set(_std_stacktrace_library stdc++exp) + else() + set(_std_stacktrace_library stdc++_libbacktrace) + endif() +elseif(CMAKE_CXX_COMPILER_ID STREQUAL "Clang") + set(_std_stacktrace_library stdc++exp) +endif() + cet_make_library( LIBRARY_NAME phlex_core @@ -34,10 +44,12 @@ cet_make_library( phlex::metaprogramming phlex::model phlex::utilities + ${_std_stacktrace_library} PRIVATE Boost::json spdlog::spdlog ) + install( FILES concepts.hpp @@ -94,6 +106,7 @@ phlex_make_internal_library( phlex_core LIBRARIES PUBLIC TBB::tbb phlex::metaprogramming phlex_model_internal phlex_utilities_internal + ${_std_stacktrace_library} PRIVATE Boost::json spdlog::spdlog ) add_library(phlex::core_internal ALIAS phlex_core_internal) diff --git a/phlex/core/declared_fold.cpp b/phlex/core/declared_fold.cpp index 98393d1c2..045f7941b 100644 --- a/phlex/core/declared_fold.cpp +++ b/phlex/core/declared_fold.cpp @@ -5,7 +5,8 @@ namespace phlex::detail { std::vector predicates, product_selectors input_products, std::string partition_layer) : - products_consumer{std::move(name), std::move(predicates), std::move(input_products)}, + products_consumer{ + std::move(name), std::move(predicates), std::move(input_products), layers_required::always}, partition_layer_{std::move(partition_layer)} { } diff --git a/phlex/core/declared_unfold.cpp b/phlex/core/declared_unfold.cpp index ed8bab807..fd64839a7 100644 --- a/phlex/core/declared_unfold.cpp +++ b/phlex/core/declared_unfold.cpp @@ -31,7 +31,8 @@ namespace phlex::detail { std::vector predicates, product_selectors input_products, std::string child_layer) : - products_consumer{std::move(name), std::move(predicates), std::move(input_products)}, + products_consumer{ + std::move(name), std::move(predicates), std::move(input_products), layers_required::always}, child_layer_{std::move(child_layer)} { } diff --git a/phlex/core/framework_graph.cpp b/phlex/core/framework_graph.cpp index 438f841f8..b2a382f2b 100644 --- a/phlex/core/framework_graph.cpp +++ b/phlex/core/framework_graph.cpp @@ -66,11 +66,9 @@ namespace phlex::detail { phlex::experimental::identifier const child_layer{n->child_layer()}; std::set input_layers_for_unfold; for (auto const& input : n->input()) { + // Existence of layer already validated auto const& input_layer = static_cast(input.layer); - if (input_layer.empty()) { - continue; - } if (not input_layers_for_unfold.insert(input_layer).second) { continue; } diff --git a/phlex/core/index_router.cpp b/phlex/core/index_router.cpp index 78b26bafe..459e68713 100644 --- a/phlex/core/index_router.cpp +++ b/phlex/core/index_router.cpp @@ -107,6 +107,7 @@ namespace phlex::detail { fold_partition_ports_t fold_partition_ports, std::map multilayer_join_ports) { + using namespace phlex::experimental::literals; // We must have at least one provider port, or there can be no data to process. assert(!provider_input_ports.empty()); @@ -197,7 +198,7 @@ namespace phlex::detail { provider_input_ports_t provider_input_ports) { for (auto& [input_product, provider_port] : provider_input_ports | std::views::values) { - auto [it, _] = index_set_nodes_.emplace(input_product.layer, + auto [it, _] = index_set_nodes_.emplace(input_product.layer ? *input_product.layer : "*"_id, std::make_shared(g)); make_edge(*it->second, *provider_port); } diff --git a/phlex/core/make_computational_edges.cpp b/phlex/core/make_computational_edges.cpp index 79fa2bade..67a450098 100644 --- a/phlex/core/make_computational_edges.cpp +++ b/phlex/core/make_computational_edges.cpp @@ -22,10 +22,24 @@ namespace phlex::detail { }; auto proj = [](auto const& pair) -> provider_node* { return pair.second.get(); }; - if (auto it = std::ranges::find_if(providers, pred, proj); it != providers.end()) { - return it->second.get(); + auto candidates = providers | std::views::transform(proj) | std::views::filter(pred) | + std::ranges::to(); + switch (candidates.size()) { + case 0: + return nullptr; + case 1: + return candidates[0]; + default: + auto items = candidates | std::views::transform([](provider_node* p) { + return fmt::format("spec: {}, layer: {}, stage: {}", + p->output_product().to_string(), + p->layer(), + p->stage()); + }); + throw std::runtime_error(fmt::format("Multiple explicit providers found for {}:\n{}", + input_product.to_string(), + bulleted_list(items))); } - return nullptr; } provider_bundles find_matching_implicit_providers(source_map const& sources, @@ -52,8 +66,12 @@ namespace phlex::detail { // output port) and the right family (hidden in the input port). if (auto* matched_provider = find_matching_provider(explicit_providers, input_product)) { auto const provider_name = matched_provider->name().to_string(); - provider_input_ports.try_emplace( + auto&& [it, _] = provider_input_ports.try_emplace( provider_name, input_product, matched_provider->input_port()); + // Rewrite the stored layer if it's empty + if (!input_product.layer) { + it->second.input_product.layer = matched_provider->layer(); + } spdlog::debug("Connecting provider {} to node {} (product: {})", provider_name, node_name, @@ -79,7 +97,7 @@ namespace phlex::detail { for (auto const& [input_product, port] : ports) { auto existing_provider_it = std::ranges::find_if( provider_input_ports, [&input_product](auto const& provider_entry) { - return provider_entry.second.input_product == input_product; + return input_product.match(provider_entry.second.input_product); }); if (existing_provider_it != provider_input_ports.end()) { @@ -116,8 +134,12 @@ namespace phlex::detail { phlex::experimental::identifier{bundle.layer}, phlex::experimental::identifier{bundle.stage}); auto const provider_name = node->name().to_string(); - auto [_, inserted] = + auto&& [it, inserted] = provider_input_ports.try_emplace(provider_name, input_product, node->input_port()); + // Rewrite the stored layer if it's empty + if (!input_product.layer) { + it->second.input_product.layer = node->layer(); + } if (!inserted) { throw std::runtime_error( fmt::format("Failed to create implicit provider for product selector '{}'\n" diff --git a/phlex/core/multilayer_join_node.hpp b/phlex/core/multilayer_join_node.hpp index 50142f33a..f98607479 100644 --- a/phlex/core/multilayer_join_node.hpp +++ b/phlex/core/multilayer_join_node.hpp @@ -76,8 +76,7 @@ namespace phlex::detail { name_{std::move(node_name)}, layers_{std::move(layer_names)} { - assert(NInputs == layers_.size()); - + using namespace experimental::literals; // Collapse to the set of distinct layer names. More than one distinct layer means // at least one input crosses a layer boundary and therefore every input stream // needs a repeater_node. diff --git a/phlex/core/product_selector.cpp b/phlex/core/product_selector.cpp index d754ffb66..a846f2c24 100644 --- a/phlex/core/product_selector.cpp +++ b/phlex/core/product_selector.cpp @@ -9,7 +9,7 @@ namespace phlex { if (creator && creator != other.creator) { return false; } - if (layer != other.layer) { + if (layer && layer != other.layer) { return false; } if (suffix && suffix != other.suffix) { @@ -50,7 +50,7 @@ namespace phlex { if (!match(spec)) { return false; } - if (experimental::identifier(this->layer) != layer) { + if (this->layer && experimental::identifier(this->layer) != layer) { return false; } if (this->stage && this->stage != stage) { @@ -77,7 +77,7 @@ namespace phlex { suffix.transform(&identifier::operator std::string_view).value_or("[ANY]"); std::string type_str = this->type.valid() ? fmt::format("<{}>", this->type) : "[UNSET TYPE]"; // will later be concept - auto layer_str = std::string_view(layer); + auto layer_str = layer ? std::string_view(layer) : "[ANY]"; std::string_view creator_str = creator ? std::string_view(*creator) : "[ANY]"; std::string_view stage_str = stage.transform(&identifier::operator std::string_view).value_or("[ANY]"); @@ -99,12 +99,8 @@ namespace phlex { std::strong_ordering product_selector::operator<=>(product_selector const& rhs) const { using experimental::identifier; - return std::tie(type, creator, static_cast(layer), suffix, stage) <=> - std::tie(rhs.type, - rhs.creator, - static_cast(rhs.layer), - rhs.suffix, - rhs.stage); + return std::tie(type, creator, layer, suffix, stage) <=> + std::tie(rhs.type, rhs.creator, rhs.layer, rhs.suffix, rhs.stage); } detail::product_specification const* resolve_in_store(product_selector const& query, diff --git a/phlex/core/product_selector.hpp b/phlex/core/product_selector.hpp index 89098b065..d81cba215 100644 --- a/phlex/core/product_selector.hpp +++ b/phlex/core/product_selector.hpp @@ -9,7 +9,9 @@ #include "phlex/model/type_id.hpp" #include +#include #include +#include #include #include #include @@ -25,6 +27,12 @@ namespace phlex { class creator_name { public: creator_name() : content_{std::nullopt} {} + creator_name(std::optional&& content) : content_{std::move(content)} + { + if (content_ && content_.value().empty()) { + throw std::runtime_error("Cannot specify product with empty creator name."); + } + } template requires std::constructible_from // NOLINTNEXTLINE(google-explicit-constructor) - Implicit conversion is intentional @@ -42,6 +50,7 @@ namespace phlex { return me.content_.value_or("[ANY]"); } bool operator==(creator_name const&) const noexcept = default; + auto operator<=>(creator_name const&) const noexcept = default; private: std::optional content_; @@ -49,35 +58,56 @@ namespace phlex { // The required_layer_name has to be a template for static_assert(false) template T> - class required_layer_name { + class layer_name { public: - required_layer_name() + layer_name() : content_(std::nullopt) {} + layer_name(std::optional&& content) : content_{std::move(content)} { - static_assert(false, "The layer name has not been set in this product_selector."); + if (content_ && content_.value().empty()) { + throw std::runtime_error("Cannot specify the empty string as a data layer."); + } } template requires std::constructible_from // NOLINTNEXTLINE(google-explicit-constructor) - Implicit conversion is intentional - required_layer_name(U&& rhs) : content_(std::forward(rhs)) + layer_name(U&& rhs) : content_(std::forward(rhs)) { - if (content_.empty()) { + if (content_.value().empty()) { throw std::runtime_error("Cannot specify the empty string as a data layer."); } } // NOLINTNEXTLINE(google-explicit-constructor) - Implicit conversion is intentional - operator T const&() const noexcept { return content_; } - explicit operator std::string_view() const noexcept { return std::string_view(content_); } - bool operator==(required_layer_name const&) const noexcept = default; + operator T const&() const + { + if (!content_.has_value()) { + throw std::logic_error( + std::format("Cannot retrieve layer from product_selector with no layer\n{}\n", + std::stacktrace::current())); + } + return content_.operator*(); + } + + experimental::identifier const& operator*() const noexcept { return content_.operator*(); } + explicit operator std::string_view() const noexcept + { + using namespace std::string_view_literals; + return content_ + .transform([](experimental::identifier const& id) { return std::string_view(id); }) + .value_or(""sv); + } + operator bool() const noexcept { return content_.has_value(); } + bool operator==(layer_name const&) const noexcept = default; + auto operator<=>(layer_name const&) const noexcept = default; private: - experimental::identifier content_; + std::optional content_; }; } struct PHLEX_CORE_EXPORT product_selector { detail::creator_name creator; - detail::required_layer_name layer; + detail::layer_name layer; std::optional suffix; std::optional stage; detail::type_id type; diff --git a/phlex/core/products_consumer.cpp b/phlex/core/products_consumer.cpp index af268bb42..b01a0e754 100644 --- a/phlex/core/products_consumer.cpp +++ b/phlex/core/products_consumer.cpp @@ -1,13 +1,21 @@ #include "phlex/core/products_consumer.hpp" +#include "fmt/format.h" + namespace { std::vector layers_from(phlex::product_selectors const& queries) { + using namespace phlex::experimental::literals; std::vector result; result.reserve(queries.size()); for (auto const& query : queries) { - result.push_back(query.layer); + if (query.layer) { + result.push_back(query.layer); + } else { + result.push_back("*"_id); + } } + result.shrink_to_fit(); return result; } } @@ -16,11 +24,33 @@ namespace phlex::detail { products_consumer::products_consumer(phlex::experimental::algorithm_name name, std::vector predicates, - product_selectors input_products) : + product_selectors input_products, + layers_required layers_required) : consumer{std::move(name), std::move(predicates)}, input_products_{std::move(input_products)}, layers_{layers_from(input_products_)} { + using namespace phlex::experimental::literals; + if (layers_required == layers_required::always || + (layers_required != layers_required::never && input_products_.size() > 1)) { + std::vector err_selectors{}; + for (auto const& p : input_products_) { + if (!p.layer) { + err_selectors.push_back(p.to_string()); + } + } + if (!err_selectors.empty()) { + std::string type = + layers_required == layers_required::always ? "layer-mandatory" : "multi-input"; + std::string error = + fmt::format("Product selectors in {} algorithm {} must define their layers:\n" + " (Only invalid selectors are listed)\n{}", + type, + this->name().to_string(), + bulleted_list(err_selectors)); + throw std::runtime_error(error); + } + } } products_consumer::~products_consumer() = default; diff --git a/phlex/core/products_consumer.hpp b/phlex/core/products_consumer.hpp index 1c77ac382..38a9bf55a 100644 --- a/phlex/core/products_consumer.hpp +++ b/phlex/core/products_consumer.hpp @@ -17,11 +17,13 @@ #include namespace phlex::detail { + enum class layers_required : char { never, multi_input_only, always }; class PHLEX_CORE_EXPORT products_consumer : public consumer { public: products_consumer(phlex::experimental::algorithm_name name, std::vector predicates, - product_selectors input_products); + product_selectors input_products, + layers_required layers_required = layers_required::multi_input_only); virtual ~products_consumer(); diff --git a/phlex/core/registrar.hpp b/phlex/core/registrar.hpp index 44b31337c..fb82d54fc 100644 --- a/phlex/core/registrar.hpp +++ b/phlex/core/registrar.hpp @@ -112,11 +112,17 @@ namespace phlex::detail { void create_node(std::vector output_product_suffixes) { assert(creator_); - auto ptr = creator_(release_predicates(), std::move(output_product_suffixes)); - auto name = ptr->name().to_string(); - auto [_, inserted] = nodes_->try_emplace(name, std::move(ptr)); - if (not inserted) { - internal::add_to_error_messages(*errors_, "Node", name); + try { + auto ptr = creator_(release_predicates(), std::move(output_product_suffixes)); + auto name = ptr->name().to_string(); + auto [_, inserted] = nodes_->try_emplace(name, std::move(ptr)); + if (not inserted) { + internal::add_to_error_messages(*errors_, "Node", name); + } + } catch (...) { + // Prevent trying to re-run create_node + creator_ = nullptr; + throw; } } diff --git a/test/configuration.cpp b/test/configuration.cpp index 195f910c4..555e3b17d 100644 --- a/test/configuration.cpp +++ b/test/configuration.cpp @@ -43,7 +43,7 @@ TEST_CASE("Retrieve product_selector", "[config]") boost::json::object malformed_input2; malformed_input2["creator"] = "hits"; - malformed_input2["level"] = "should be layer, not level"; + malformed_input2["layer"] = ""; boost::json::object underlying_config; underlying_config["input"] = std::move(input); @@ -59,5 +59,5 @@ TEST_CASE("Retrieve product_selector", "[config]") ContainsSubstring("not a string")); CHECK_THROWS_WITH(config.get("malformed2"), ContainsSubstring("Error retrieving parameter 'malformed2'") && - ContainsSubstring("Error retrieving parameter 'layer'")); + ContainsSubstring("Cannot specify the empty string as a data layer.")); } diff --git a/test/product_selecting.cpp b/test/product_selecting.cpp index efd4f0478..d01f34b34 100644 --- a/test/product_selecting.cpp +++ b/test/product_selecting.cpp @@ -1,8 +1,10 @@ #include "phlex/core/framework_graph.hpp" #include "phlex/model/data_cell_index.hpp" #include "phlex/model/product_store.hpp" +#include "phlex/source.hpp" #include "catch2/catch_test_macros.hpp" +#include "catch2/matchers/catch_matchers_string.hpp" #include "plugins/layer_generator.hpp" #include "fmt/format.h" @@ -26,6 +28,42 @@ namespace { { return fmt::format("John the {}th", dci.number()); } + + detail::product_ptr provide_archived_count(data_cell_index const& dci) + { + return std::make_unique>(static_cast(dci.number())); + } + + class archived_count_source : public source { + public: + detail::provider_bundles create_providers(product_selector const& selector) override + { + using namespace experimental::literals; + phlex::detail::product_specification spec{ + "archived_input", "archived_count", phlex::detail::make_type_id()}; + if (!selector.match(spec, "event"_id, "previous_process"_id)) { + return {}; + } + return {{.provider_function = provide_archived_count, + .max_concurrency = concurrency::unlimited, + .spec = std::move(spec), + .layer = "event", + .stage = "previous_process"}}; + } + + index_generator indices() override { co_return; } + }; + + class copy_temperature_once { + public: + explicit copy_temperature_once(double const temperature) : temperature_{temperature} {} + bool initial_value() const { return true; } + bool predicate(bool const emit) const { return emit; } + auto unfold(bool const) const { return std::pair{false, temperature_}; } + + private: + double temperature_; + }; } TEST_CASE("Querying products in different ways", "[graph]") @@ -35,6 +73,7 @@ TEST_CASE("Querying products in different ways", "[graph]") gen->add_layer("event", {.parent_layer_name = "job", .total_per_parent_data_cell = num_events}); auto g = phlex::detail::framework_graph::without_driver(); g.add_driver(gen); + g.add_source("archived_count_source"); // Register providers g.provide("provide_number_in_job", provide_number, concurrency::unlimited) @@ -50,21 +89,21 @@ TEST_CASE("Querying products in different ways", "[graph]") // Duplicate with transform g.transform("duplicate_temperature", [](double const& t) { return t; }) - .input_family(product_selector{.creator = "input", .layer = "event", .suffix = "temperature"}) + .input_family(product_selector{.creator = "input", .suffix = "temperature"}) .output_product_suffixes("temperature"); - SECTION("All fields") + SECTION("Creator and suffix without layer") { - g.transform("all_fields", [](int const& i) { return i + 1; }) - .input_family(product_selector{.creator = "input", .layer = "job", .suffix = "number"}) + g.transform("creator_and_suffix_without_layer", [](int const& i) { return i + 1; }) + .input_family(product_selector{.creator = "input", .suffix = "number"}) .output_product_suffixes("job_number"); g.execute(); - CHECK(g.execution_count("all_fields") == 1); + CHECK(g.execution_count("creator_and_suffix_without_layer") == 1); } - SECTION("Creator and Layer, using creator (and using type alone)") + SECTION("Creator and layer, distinguished by type") { - g.transform("creator_and_layer_by_creator", [](std::string const& str) { return str; }) + g.transform("name_by_creator_and_layer", [](std::string const& str) { return str; }) .input_family(product_selector{.creator = "give_name", .layer = "event"}) .output_product_suffixes("event_name"); g.observe( @@ -73,33 +112,84 @@ TEST_CASE("Querying products in different ways", "[graph]") .input_family(product_selector{.creator = "give_name", .layer = "event"}, product_selector{.creator = "input", .layer = "event"}); g.execute(); - CHECK(g.execution_count("creator_and_layer_by_creator") == num_events); + CHECK(g.execution_count("name_by_creator_and_layer") == num_events); } SECTION("Layer alone, distinguished by type") { - g.transform("layer_by_type", [](std::string const& str) { return str; }) + g.transform("name_by_layer_and_type", [](std::string const& str) { return str; }) .input_family(product_selector{.layer = "event"}) .output_product_suffixes("new_name"); g.execute(); - CHECK(g.execution_count("layer_by_type") == num_events); + CHECK(g.execution_count("name_by_layer_and_type") == num_events); } - SECTION("Creator and Layer, using layer") + SECTION("Creator only without layer") { - g.transform("creator_and_layer_by_layer", [](double const& d) { return d; }) - .input_family(product_selector{.creator = "input", .layer = "event"}) + g.transform("temperature_by_creator_without_layer", [](double const& d) { return d; }) + .input_family(product_selector{.creator = "input"}) .output_product_suffixes("event_temp"); g.execute(); - CHECK(g.execution_count("creator_and_layer_by_layer") == num_events); + CHECK(g.execution_count("temperature_by_creator_without_layer") == num_events); } - SECTION("Creator and Layer only, after transform") + SECTION("Creator only without layer, after transform") { - g.transform("creator_and_layer_after_transform", [](double const& d) { return d; }) - .input_family(product_selector{.creator = "duplicate_temperature", .layer = "event"}) + g.transform("temperature_from_transform_without_layer", [](double const& d) { return d; }) + .input_family(product_selector{.creator = "duplicate_temperature"}) .output_product_suffixes("event_temp"); g.execute(); - CHECK(g.execution_count("creator_and_layer_after_transform") == num_events); + CHECK(g.execution_count("temperature_from_transform_without_layer") == num_events); + } + + SECTION("Predicate and observer inputs without layer") + { + g.predicate( + "even_event_temperature", + [](double const temperature) { return static_cast(temperature / 100.0) % 2 == 0; }, + concurrency::unlimited) + .input_family(product_selector{.creator = "input", .suffix = "temperature"}); + g.observe( + "observe_even_event_temperature", [](double const) {}, concurrency::unlimited) + .input_family(product_selector{.creator = "input", .suffix = "temperature"}) + .experimental_when("even_event_temperature"); + g.execute(); + CHECK(g.execution_count("even_event_temperature") == num_events); + CHECK(g.execution_count("observe_even_event_temperature") == 13); + } + + SECTION("Unfold inputs without layer are rejected") + { + CHECK_THROWS_WITH( + g.unfold("copy_temperature_once", + ©_temperature_once::predicate, + ©_temperature_once::unfold, + concurrency::unlimited, + "temperature_copy") + .input_family(product_selector{.creator = "input", .suffix = "temperature"}) + .output_product_suffixes("temperature"), + "Product selectors in layer-mandatory algorithm copy_temperature_once must define their " + "layers:\n" + " (Only invalid selectors are listed)\n" + " - ) by creator input (of stage [ANY]) in layer " + "[ANY]>"); + } + + SECTION("Shared implicit provider inputs without layer") + { + auto const archived_count = + product_selector{.creator = "archived_input", .suffix = "archived_count"}; + g.transform("copy_archived_count", [](int const count) { return count; }) + .input_family(archived_count) + .output_product_suffixes("archived_count_copy"); + g.observe( + "observe_archived_count", + [](handle const count) { CHECK(count.stage() == "previous_process"); }, + concurrency::unlimited) + .input_family(archived_count); + g.execute(); + CHECK(g.execution_count("archived_input") == num_events); + CHECK(g.execution_count("copy_archived_count") == num_events); + CHECK(g.execution_count("observe_archived_count") == num_events); } }