diff --git a/phlex/app/load_module.cpp b/phlex/app/load_module.cpp index e7703474f..5843090f1 100644 --- a/phlex/app/load_module.cpp +++ b/phlex/app/load_module.cpp @@ -110,11 +110,12 @@ namespace phlex::experimental { creator(g.source_proxy(config), config); } - detail::next_index_t load_driver(boost::json::object const& raw_config) + driver_bundle load_driver(boost::json::object const& raw_config) { configuration const config{raw_config}; auto const& spec = config.get("cpp"); create_driver = plugin_loader(spec, "create_driver"); - return create_driver(config); + driver_proxy const proxy{}; + return create_driver(proxy, config); } } diff --git a/phlex/app/load_module.hpp b/phlex/app/load_module.hpp index 14d3ba1f4..5b524a324 100644 --- a/phlex/app/load_module.hpp +++ b/phlex/app/load_module.hpp @@ -6,8 +6,6 @@ #include "boost/json.hpp" -#include - namespace phlex::experimental { namespace detail { // Adjust_config adds the module_label as a parameter, and it checks if the 'py' @@ -17,7 +15,7 @@ namespace phlex::experimental { void load_module(framework_graph& g, std::string const& label, boost::json::object config); void load_source(framework_graph& g, std::string const& label, boost::json::object config); - detail::next_index_t load_driver(boost::json::object const& config); + driver_bundle load_driver(boost::json::object const& config); } #endif // PHLEX_APP_LOAD_MODULE_HPP diff --git a/phlex/core/framework_graph.cpp b/phlex/core/framework_graph.cpp index c388dcef9..3e32feb69 100644 --- a/phlex/core/framework_graph.cpp +++ b/phlex/core/framework_graph.cpp @@ -13,24 +13,29 @@ #include namespace phlex::experimental { - framework_graph::framework_graph(data_cell_index_ptr index, int const max_parallelism) : - framework_graph{[index](framework_driver& driver) { driver.yield(index); }, max_parallelism} + framework_graph::framework_graph(int const max_parallelism) : + framework_graph{[](framework_driver& driver) { driver.yield(data_cell_index::job()); }, + max_parallelism} { } framework_graph::framework_graph(detail::next_index_t next_index, int const max_parallelism) : + framework_graph{driver_bundle{std::move(next_index), {}}, max_parallelism} + { + } + + framework_graph::framework_graph(driver_bundle bundle, int const max_parallelism) : parallelism_limit_{static_cast(max_parallelism)}, - driver_{std::move(next_index)}, + fixed_hierarchy_{std::move(bundle.hierarchy)}, + driver_{std::move(bundle.driver)}, src_{graph_, [this](tbb::flow_control& fc) mutable -> data_cell_index_ptr { - auto item = driver_(); - if (not item) { - index_router_.drain(); - fc.stop(); - return {}; + if (auto item = driver_()) { + return index_router_.route(*item); } - - return index_router_.route(*item); + index_router_.drain(); + fc.stop(); + return {}; }}, index_router_{graph_}, hierarchy_node_{graph_, @@ -175,5 +180,4 @@ namespace phlex::experimental { make_edge(node->output_index_port(), hierarchy_node_); } } - } diff --git a/phlex/core/framework_graph.hpp b/phlex/core/framework_graph.hpp index 49a1a6c04..6dd8a4816 100644 --- a/phlex/core/framework_graph.hpp +++ b/phlex/core/framework_graph.hpp @@ -33,9 +33,10 @@ namespace phlex { namespace phlex::experimental { class framework_graph { public: - explicit framework_graph(data_cell_index_ptr index, + explicit framework_graph(int max_parallelism = oneapi::tbb::info::default_concurrency()); + explicit framework_graph(detail::next_index_t next_index, int max_parallelism = oneapi::tbb::info::default_concurrency()); - explicit framework_graph(detail::next_index_t f, + explicit framework_graph(driver_bundle bundle, int max_parallelism = oneapi::tbb::info::default_concurrency()); ~framework_graph(); @@ -151,6 +152,7 @@ namespace phlex::experimental { resource_usage graph_resource_usage_{}; max_allowed_parallelism parallelism_limit_; + fixed_hierarchy fixed_hierarchy_; data_layer_hierarchy hierarchy_{}; node_catalog nodes_{}; std::map filters_{}; diff --git a/phlex/detail/plugin_macros.hpp b/phlex/detail/plugin_macros.hpp index 9abc057e9..1a3e82802 100644 --- a/phlex/detail/plugin_macros.hpp +++ b/phlex/detail/plugin_macros.hpp @@ -22,4 +22,23 @@ BOOST_DLL_ALIAS(func_name, dll_alias) \ PHLEX_DETAIL_SELECT_SIGNATURE(token_type, func_name, __VA_ARGS__) +#define PHLEX_DETAIL_CREATE_DRIVER_1ARG(func_name, d) \ + phlex::experimental::driver_bundle func_name(phlex::experimental::driver_proxy const& d, \ + phlex::configuration const&) + +#define PHLEX_DETAIL_CREATE_DRIVER_2ARGS(func_name, d, config) \ + phlex::experimental::driver_bundle func_name(phlex::experimental::driver_proxy const& d, \ + phlex::configuration const& config) + +#define PHLEX_DETAIL_SELECT_DRIVER_SIGNATURE(func_name, ...) \ + BOOST_PP_IF(BOOST_PP_EQUAL(PHLEX_DETAIL_NARGS(__VA_ARGS__), 1), \ + PHLEX_DETAIL_CREATE_DRIVER_1ARG, \ + PHLEX_DETAIL_CREATE_DRIVER_2ARGS) \ + (func_name, __VA_ARGS__) + +#define PHLEX_DETAIL_REGISTER_DRIVER_PLUGIN(func_name, dll_alias, ...) \ + static PHLEX_DETAIL_SELECT_DRIVER_SIGNATURE(func_name, __VA_ARGS__); \ + BOOST_DLL_ALIAS(func_name, dll_alias) \ + PHLEX_DETAIL_SELECT_DRIVER_SIGNATURE(func_name, __VA_ARGS__) + #endif // PHLEX_DETAIL_PLUGIN_MACROS_HPP diff --git a/phlex/driver.hpp b/phlex/driver.hpp index e190180fa..6e9e80934 100644 --- a/phlex/driver.hpp +++ b/phlex/driver.hpp @@ -5,70 +5,51 @@ #include "phlex/configuration.hpp" #include "phlex/core/fwd.hpp" +#include "phlex/detail/plugin_macros.hpp" +#include "phlex/model/data_cell_index.hpp" +#include "phlex/model/fixed_hierarchy.hpp" #include "phlex/model/product_store.hpp" #include "phlex/utilities/async_driver.hpp" #include -#include +#include +#include -namespace phlex { - using framework_driver = experimental::async_driver; -} - -namespace phlex::experimental::detail { +namespace phlex::experimental { + class driver_proxy; + struct driver_bundle; - // See note below. - template - auto make(configuration const& config) - { - if constexpr (requires { T{config}; }) { - return std::make_shared(config); - } else { - return std::make_shared(); - } - } + using framework_driver = experimental::async_driver; - template - concept next_function_with_driver = requires(T t, framework_driver& driver) { - { t.next(driver) } -> std::same_as; + namespace detail { + using next_index_t = std::function; + using driver_creator_t = driver_bundle(driver_proxy const&, configuration const&); }; - template - concept next_function_without_driver = requires(T t) { - { t.next() } -> std::same_as; + struct driver_bundle { + detail::next_index_t driver; + fixed_hierarchy hierarchy; }; +} - // Workaround for static_assert(false) until P2593R1 is adopted - // https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2023/p2593r1.html - // static_assert(false) is supported in GCC 13 and newer - template - constexpr bool always_false{false}; - - template - std::function create_next(configuration const& config = {}) - { - // N.B. Because we are initializing an std::function object with a lambda, the lambda - // (and therefore its captured values) must be copy-constructible. This means - // that make(config) must return a copy-constructible object. Because we do not - // know if a user's provided driver class is copyable, we create the object on - // the heap, and capture a shared pointer to the object. This also ensures that - // the driver object is created only once, thus avoiding potential errors in the - // implementations of the driver class' copy/move constructors (e.g. if the - // source is caching an iterator). - if constexpr (next_function_with_driver) { - return [t = make(config)](framework_driver& driver) { t->next(driver); }; - } else if constexpr (next_function_without_driver) { - return [t = make(config)](framework_driver&) { t->next(); }; - } else { - static_assert(always_false, "Must have a 'next()' function that returns 'void'"); +namespace phlex::experimental { + template + concept is_driver_like = std::invocable; + + class driver_proxy { + public: + driver_bundle driver(fixed_hierarchy hierarchy, is_driver_like auto driver_function) const + { + auto h = hierarchy; + return {[f = std::move(driver_function), h = std::move(h)](framework_driver& d) mutable { + f(h.yield_job(d)); + }, + std::move(hierarchy)}; } - } - - using next_index_t = std::function; - using driver_creator_t = next_index_t(configuration const&); + }; } -#define PHLEX_EXPERIMENTAL_REGISTER_DRIVER(driver) \ - BOOST_DLL_ALIAS(phlex::experimental::detail::create_next, create_driver) +#define PHLEX_EXPERIMENTAL_REGISTER_DRIVER(...) \ + PHLEX_DETAIL_REGISTER_DRIVER_PLUGIN(create, create_driver, __VA_ARGS__) #endif // PHLEX_DRIVER_HPP diff --git a/phlex/model/CMakeLists.txt b/phlex/model/CMakeLists.txt index 6b5eabf27..3e196254b 100644 --- a/phlex/model/CMakeLists.txt +++ b/phlex/model/CMakeLists.txt @@ -5,6 +5,7 @@ cet_make_library( SOURCE algorithm_name.cpp data_cell_counter.cpp + fixed_hierarchy.cpp data_layer_hierarchy.cpp data_cell_index.cpp identifier.cpp @@ -25,6 +26,7 @@ cet_make_library( install( FILES algorithm_name.hpp + fixed_hierarchy.hpp fwd.hpp handle.hpp data_cell_counter.hpp diff --git a/phlex/model/data_cell_index.cpp b/phlex/model/data_cell_index.cpp index a83e22df8..371e80ddd 100644 --- a/phlex/model/data_cell_index.cpp +++ b/phlex/model/data_cell_index.cpp @@ -51,11 +51,10 @@ namespace phlex { // FIXME: Should it be an error to create an ID with an empty name? } - data_cell_index const& data_cell_index::base() { return *base_ptr(); } - data_cell_index_ptr data_cell_index::base_ptr() + data_cell_index_ptr data_cell_index::job() { - static data_cell_index_ptr base_id{new data_cell_index}; - return base_id; + static data_cell_index_ptr job_index{new data_cell_index}; + return job_index; } experimental::identifier const& data_cell_index::layer_name() const noexcept diff --git a/phlex/model/data_cell_index.hpp b/phlex/model/data_cell_index.hpp index e96d4b7cb..9c353ab32 100644 --- a/phlex/model/data_cell_index.hpp +++ b/phlex/model/data_cell_index.hpp @@ -16,8 +16,7 @@ namespace phlex { class data_cell_index : public std::enable_shared_from_this { public: - static data_cell_index const& base(); - static data_cell_index_ptr base_ptr(); + static data_cell_index_ptr job(); using hash_type = std::size_t; data_cell_index_ptr make_child(std::string layer_name, std::size_t data_cell_number) const; diff --git a/phlex/model/fixed_hierarchy.cpp b/phlex/model/fixed_hierarchy.cpp new file mode 100644 index 000000000..872bb24af --- /dev/null +++ b/phlex/model/fixed_hierarchy.cpp @@ -0,0 +1,108 @@ +#include "phlex/model/fixed_hierarchy.hpp" + +#include "phlex/model/data_cell_index.hpp" +#include "phlex/model/identifier.hpp" +#include "phlex/utilities/async_driver.hpp" +#include "phlex/utilities/hashing.hpp" + +#include "fmt/format.h" + +#include +#include +#include +#include +#include + +namespace { + // Each path must be non-empty and may only contain "job" as the first element. + std::span validated_path(std::vector const& path) + { + if (path.empty()) { + throw std::runtime_error("Layer paths cannot be empty."); + } + auto const rest = std::span{path}.subspan(path[0] == "job" ? 1 : 0); + if (std::ranges::contains(rest, "job")) { + throw std::runtime_error("Layer paths may only contain 'job' as the first element."); + } + return rest; + } + + // Builds the set of cumulative layer hashes that define the fixed hierarchy. + // For example, if the layer paths are ["job", "run", "subrun"] and ["job", "spill"], + // the hashes included will correspond to: + // From the first path: + // - "job" + // - "job/run" + // - "job/run/subrun" + // + // From the second path: + // - "job" (already included from the first path) + // - "job/spill" + // + // Each path must be non-empty and may only contain "job" as the first element. + std::set build_hashes(std::vector> const& layer_paths) + { + using namespace phlex::experimental; + identifier const job{"job"}; + std::set hashes{job.hash()}; + for (std::vector const& path : layer_paths) { + std::size_t cumulative_hash = job.hash(); + for (auto const& name : validated_path(path)) { + cumulative_hash = hash(cumulative_hash, identifier{name}.hash()); + hashes.insert(cumulative_hash); + } + } + return hashes; + } +} + +namespace phlex::experimental { + + data_cell_cursor::data_cell_cursor(data_cell_index_ptr index, + fixed_hierarchy const& h, + async_driver& d) : + index_{std::move(index)}, hierarchy_{h}, driver_{d} + { + } + + data_cell_cursor data_cell_cursor::yield_child(std::string const& layer_name, + std::size_t number) const + { + auto child = index_->make_child(layer_name, number); + hierarchy_.validate(child); + driver_.yield(child); + return data_cell_cursor{child, hierarchy_, driver_}; + } + + std::string data_cell_cursor::layer_path() const { return index_->layer_path(); } + + fixed_hierarchy::fixed_hierarchy(std::initializer_list> layer_paths) : + fixed_hierarchy(std::vector>(layer_paths)) + { + } + + fixed_hierarchy::fixed_hierarchy(std::vector> layer_paths) : + layer_hashes_(std::from_range, build_hashes(layer_paths)) + { + } + + void fixed_hierarchy::validate(data_cell_index_ptr const& index) const + { + if (layer_hashes_.empty()) { + return; + } + if (std::ranges::binary_search(layer_hashes_, index->layer_hash())) { + return; + } + throw std::runtime_error( + fmt::format("Layer {} is not part of the fixed hierarchy.", index->layer_path())); + } + + data_cell_cursor fixed_hierarchy::yield_job(async_driver& d) const + { + auto job = data_cell_index::job(); + d.yield(job); + return data_cell_cursor{job, *this, d}; + } + +} diff --git a/phlex/model/fixed_hierarchy.hpp b/phlex/model/fixed_hierarchy.hpp new file mode 100644 index 000000000..caf9614dc --- /dev/null +++ b/phlex/model/fixed_hierarchy.hpp @@ -0,0 +1,59 @@ +#ifndef PHLEX_MODEL_FIXED_HIERARCHY_HPP +#define PHLEX_MODEL_FIXED_HIERARCHY_HPP + +#include "phlex/model/fwd.hpp" + +#include +#include +#include +#include + +namespace phlex::experimental { + template + class async_driver; +} + +namespace phlex::experimental { + + class fixed_hierarchy; + + class data_cell_cursor { + public: + // Validates that the child layer is part of the fixed hierarchy and yields the child + // data-cell index to the underlying driver, returning a data_cell_cursor for the child. + data_cell_cursor yield_child(std::string const& layer_name, std::size_t number) const; + + std::string layer_path() const; + + private: + friend class fixed_hierarchy; + data_cell_cursor(data_cell_index_ptr index, + fixed_hierarchy const& h, + async_driver& d); + + data_cell_index_ptr index_; + fixed_hierarchy const& hierarchy_; + async_driver& driver_; + }; + + class fixed_hierarchy { + public: + fixed_hierarchy() = default; + // Using an std::initializer_list removes one set of braces that the user must provide + explicit fixed_hierarchy(std::initializer_list> layer_paths); + explicit fixed_hierarchy(std::vector> layer_paths); + + void validate(data_cell_index_ptr const& index) const; + + // Yields the job-level data-cell index to the provided driver and returns a + // data_cell_cursor for the job. Must only be called from a function registered + // via driver_proxy::drive(). + data_cell_cursor yield_job(async_driver& d) const; + + private: + std::vector layer_hashes_; + }; + +} + +#endif // PHLEX_MODEL_FIXED_HIERARCHY_HPP diff --git a/phlex/model/product_store.cpp b/phlex/model/product_store.cpp index 437c0e0bc..480dbe582 100644 --- a/phlex/model/product_store.cpp +++ b/phlex/model/product_store.cpp @@ -17,7 +17,7 @@ namespace phlex::experimental { product_store_ptr product_store::base(algorithm_name base_name) { - return product_store_ptr{new product_store{data_cell_index::base_ptr(), std::move(base_name)}}; + return product_store_ptr{new product_store{data_cell_index::job(), std::move(base_name)}}; } identifier const& product_store::layer_name() const noexcept { return id_->layer_name(); } diff --git a/plugins/generate_layers.cpp b/plugins/generate_layers.cpp index 5fbffdb85..e044c9a3c 100644 --- a/plugins/generate_layers.cpp +++ b/plugins/generate_layers.cpp @@ -19,37 +19,23 @@ #include "phlex/driver.hpp" #include "plugins/layer_generator.hpp" -#include "phlex/core/framework_graph.hpp" - -#include "fmt/ranges.h" -#include "spdlog/spdlog.h" - #include -using namespace phlex; +PHLEX_EXPERIMENTAL_REGISTER_DRIVER(d, config) +{ + using namespace phlex; -namespace { - class generate_layers { - public: - generate_layers(configuration const& config) - { - auto const layers = config.get("layers", {}); - for (auto const& key : layers.keys()) { - auto const layer_config = layers.get(key); - auto const parent = layer_config.get("parent", "job"); - auto const total_number = layer_config.get("total"); - auto const starting_number = layer_config.get("starting_number", 0); - gen_.add_layer(key, {parent, total_number, starting_number}); - } + auto gen = std::make_shared(); - // FIXME: Print out statement? - } + auto const layers = config.get("layers", {}); + for (auto const& key : layers.keys()) { + auto const layer_config = layers.get(key); + gen->add_layer(key, + {.parent_layer_name = layer_config.get("parent", "job"), + .total_per_parent_data_cell = layer_config.get("total"), + .starting_value = layer_config.get("starting_number", 0)}); + } - void next(framework_driver& driver) { gen_(driver); } - - private: - experimental::layer_generator gen_; - }; + return d.driver(gen->hierarchy(), + [gen](experimental::data_cell_cursor const& job) { (*gen)(job); }); } - -PHLEX_EXPERIMENTAL_REGISTER_DRIVER(generate_layers) diff --git a/plugins/layer_generator.cpp b/plugins/layer_generator.cpp index d81be9577..794b88e3d 100644 --- a/plugins/layer_generator.cpp +++ b/plugins/layer_generator.cpp @@ -16,6 +16,19 @@ namespace phlex::experimental { emitted_cells_["/job"] = 0ull; } + fixed_hierarchy layer_generator::hierarchy() const + { + using layer_path_t = std::vector; + return fixed_hierarchy{layer_paths_ | std::views::transform([](auto const& path) { + return path | std::views::split('/') | + std::views::filter([](auto t) { return !t.empty(); }) | + std::views::transform( + [](auto t) { return std::string(t.begin(), t.end()); }) | + std::ranges::to(); + }) | + std::ranges::to>()}; + } + std::size_t layer_generator::emitted_cell_count(std::string layer_path) const { // Check if the count of all emitted cells is requested @@ -109,27 +122,27 @@ namespace phlex::experimental { layer_paths_.push_back(full_path); } - void layer_generator::execute(framework_driver& driver, data_cell_index_ptr index, bool recurse) + void layer_generator::operator()(data_cell_cursor const& job) { - auto cell_it = emitted_cells_.find(index->layer_path()); - assert(cell_it != emitted_cells_.cend()); - ++cell_it->second; - - driver.yield(index); - - if (not recurse) { - return; - } + ++emitted_cells_.at("/job"); + execute(job); + } - auto it = parent_to_children_.find(index->layer_path()); + void layer_generator::execute(data_cell_cursor const& cell) + { + auto it = parent_to_children_.find(cell.layer_path()); assert(it != parent_to_children_.cend()); for (auto const& child : it->second) { - auto const full_child_path = index->layer_path() + "/" + child; - bool const recurse = parent_to_children_.contains(full_child_path); + auto const full_child_path = cell.layer_path() + "/" + child; auto const& [_, total_per_parent, starting_value] = layers_.at(full_child_path); + bool const has_children = parent_to_children_.contains(full_child_path); for (unsigned int i : std::views::iota(starting_value, total_per_parent + starting_value)) { - execute(driver, index->make_child(child, i), recurse); + ++emitted_cells_.at(full_child_path); + auto const child_cell = cell.yield_child(child, i); + if (has_children) { + execute(child_cell); + } } } } diff --git a/plugins/layer_generator.hpp b/plugins/layer_generator.hpp index ec99f306e..d019de31b 100644 --- a/plugins/layer_generator.hpp +++ b/plugins/layer_generator.hpp @@ -28,10 +28,13 @@ // more general DAGs, where a data layer may have more than one parent. // ============================================================================================== -#include "phlex/core/framework_graph.hpp" +#include "phlex/driver.hpp" #include "phlex/model/data_cell_index.hpp" +#include +#include #include +#include namespace phlex::experimental { struct layer_spec { @@ -51,12 +54,13 @@ namespace phlex::experimental { void add_layer(std::string layer_name, layer_spec lspec); - void operator()(framework_driver& driver) { execute(driver, data_cell_index::base_ptr()); } + void operator()(data_cell_cursor const& job); + fixed_hierarchy hierarchy() const; std::size_t emitted_cell_count(std::string layer_path = {}) const; private: - void execute(framework_driver& driver, data_cell_index_ptr index, bool recurse = true); + void execute(data_cell_cursor const& cell); std::string parent_path(std::string const& layer_name, std::string const& parent_layer_spec) const; void maybe_rebase_layer_paths(std::string const& layer_name, @@ -70,10 +74,12 @@ namespace phlex::experimental { reverse_map_t parent_to_children_; }; - // N.B. The layer_generator object must outlive any whatever uses it. - std::function driver_for_test(layer_generator& generator) + // N.B. The layer_generator object must outlive whatever uses it. + inline driver_bundle driver_for_test(layer_generator& generator) { - return [&generator](framework_driver& driver) mutable { generator(driver); }; + driver_proxy const proxy{}; + return proxy.driver(generator.hierarchy(), + [&generator](data_cell_cursor const& job) { generator(job); }); } } diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index e1ddcd582..7de0cdcc4 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -93,6 +93,7 @@ cet_test( LIBRARIES phlex::core Boost::json + layer_generator ) cet_test( framework_graph @@ -161,6 +162,9 @@ cet_test( cet_test(data_cell_index USE_CATCH2_MAIN SOURCE data_cell_index.cpp LIBRARIES phlex::model ) +cet_test(fixed_hierarchy_test USE_CATCH2_MAIN SOURCE fixed_hierarchy_test.cpp LIBRARIES + phlex::model +) cet_test(product_handle USE_CATCH2_MAIN SOURCE product_handle.cpp LIBRARIES phlex::core ) @@ -233,6 +237,7 @@ cet_test( LIBRARIES spdlog::spdlog phlex::core + layer_generator ) cet_test( unfold diff --git a/test/allowed_families.cpp b/test/allowed_families.cpp index 79cd0e43f..673f8b35a 100644 --- a/test/allowed_families.cpp +++ b/test/allowed_families.cpp @@ -39,7 +39,7 @@ TEST_CASE("Testing families", "[data model]") gen.add_layer("subrun", {"run", 1}); gen.add_layer("event", {"subrun", 1}); - experimental::framework_graph g{driver_for_test(gen), 2}; + experimental::framework_graph g{driver_for_test(gen)}; // Wire up providers for each level g.provide("run_id_provider", provide_index, concurrency::unlimited) diff --git a/test/class_registration.cpp b/test/class_registration.cpp index 8fedbd54f..8ef26412b 100644 --- a/test/class_registration.cpp +++ b/test/class_registration.cpp @@ -65,7 +65,7 @@ TEST_CASE("Call non-framework functions", "[programming model]") product_query{.creator = "input", .layer = "job", .suffix = "name"}}; std::array const product_suffixes{"onumber"s, "otemperature"s, "oname"s}; - experimental::framework_graph g{data_cell_index::base_ptr()}; + experimental::framework_graph g; // Register providers for the input products g.provide("provide_number", provide_number, concurrency::unlimited) diff --git a/test/data_cell_counting.cpp b/test/data_cell_counting.cpp index 662cf89d3..0f0b90272 100644 --- a/test/data_cell_counting.cpp +++ b/test/data_cell_counting.cpp @@ -35,7 +35,7 @@ TEST_CASE("Data layer hierarchy with ambiguous layer names", "[data model]") CHECK_THROWS(h.count_for("/job")); CHECK(h.count_for("/job", true) == 0); - auto job_index = data_cell_index::base_ptr(); + auto job_index = data_cell_index::job(); h.increment_count(job_index); CHECK(h.count_for("/job") == 1); @@ -82,7 +82,7 @@ TEST_CASE("Counter multiple layers deep", "[data model]") auto const subrun_hash_value = hash(run_hash_value, "subrun"_idq.hash); auto const event_hash_value = hash(subrun_hash_value, "event"_idq.hash); - auto job_index = data_cell_index::base_ptr(); + auto job_index = data_cell_index::job(); counters.update(job_index); for (std::size_t i = 0; i != nruns; ++i) { auto run_index = job_index->make_child("run", i); diff --git a/test/data_cell_index.cpp b/test/data_cell_index.cpp index 07ed0f55c..950a6770e 100644 --- a/test/data_cell_index.cpp +++ b/test/data_cell_index.cpp @@ -10,7 +10,7 @@ TEST_CASE("Verify independent hashes", "[data model]") // In the original implementation of the hash algorithm, there was a collision between the hash for // "run:0 subrun:0 event: 760" and "run:0 subrun:1 event: 4999". - auto base = data_cell_index::base_ptr(); + auto base = data_cell_index::job(); CHECK(base->hash() == 0ull); auto run = base->make_child("run", 0); @@ -27,7 +27,7 @@ TEST_CASE("Verify independent hashes", "[data model]") TEST_CASE("data_cell_index methods", "[data model]") { - auto base = data_cell_index::base_ptr(); + auto base = data_cell_index::job(); auto run0 = base->make_child("run", 0); auto run1 = base->make_child("run", 1); @@ -68,6 +68,4 @@ TEST_CASE("data_cell_index methods", "[data model]") auto subrun = run0->make_child("subrun", 5); CHECK(subrun->layer_path() == "/job/run/subrun"); } - - SECTION("Base access") { CHECK(&data_cell_index::base() == data_cell_index::base_ptr().get()); } } diff --git a/test/filter.cpp b/test/filter.cpp index 36bfb9676..a37565905 100644 --- a/test/filter.cpp +++ b/test/filter.cpp @@ -1,6 +1,7 @@ #include "phlex/core/framework_graph.hpp" #include "phlex/model/data_cell_index.hpp" #include "phlex/model/product_store.hpp" +#include "plugins/layer_generator.hpp" #include "catch2/catch_test_macros.hpp" #include "oneapi/tbb/concurrent_vector.h" @@ -11,25 +12,6 @@ using namespace phlex; using namespace oneapi::tbb; namespace { - class source { - public: - explicit source(unsigned const max_n) : max_{max_n} {} - - void operator()(framework_driver& driver) - { - auto job_index = data_cell_index::base_ptr(); - driver.yield(job_index); - - for (unsigned int i : std::views::iota(1u, max_ + 1)) { - auto index = job_index->make_child("event", i); - driver.yield(index); - } - } - - private: - unsigned const max_; - }; - // Provider algorithms unsigned int give_me_nums(data_cell_index const& id) { return id.number() - 1; } @@ -103,7 +85,9 @@ namespace { TEST_CASE("Two predicates", "[filtering]") { - experimental::framework_graph g{source{10u}}; + experimental::layer_generator gen; + gen.add_layer("event", {"job", 10, 1}); + experimental::framework_graph g{driver_for_test(gen)}; g.provide("provide_num", give_me_nums, concurrency::unlimited) .output_product(product_query{.creator = "input", .layer = "event", .suffix = "num"}); g.predicate("evens_only", evens_only, concurrency::unlimited) @@ -127,7 +111,9 @@ TEST_CASE("Two predicates", "[filtering]") TEST_CASE("Two predicates in series", "[filtering]") { - experimental::framework_graph g{source{10u}}; + experimental::layer_generator gen; + gen.add_layer("event", {"job", 10, 1}); + experimental::framework_graph g{driver_for_test(gen)}; g.provide("provide_num", give_me_nums, concurrency::unlimited) .output_product(product_query{.creator = "input", .layer = "event", .suffix = "num"}); g.predicate("evens_only", evens_only, concurrency::unlimited) @@ -147,7 +133,9 @@ TEST_CASE("Two predicates in series", "[filtering]") TEST_CASE("Two predicates in parallel", "[filtering]") { - experimental::framework_graph g{source{10u}}; + experimental::layer_generator gen; + gen.add_layer("event", {"job", 10, 1}); + experimental::framework_graph g{driver_for_test(gen)}; g.provide("provide_num", give_me_nums, concurrency::unlimited) .output_product(product_query{.creator = "input", .layer = "event", .suffix = "num"}); g.predicate("evens_only", evens_only, concurrency::unlimited) @@ -175,7 +163,9 @@ TEST_CASE("Three predicates in parallel", "[filtering]") {.name = "exclude_6_to_7", .begin = 6, .end = 7}, {.name = "exclude_gt_8", .begin = 8, .end = -1u}}; - experimental::framework_graph g{source{10u}}; + experimental::layer_generator gen; + gen.add_layer("event", {"job", 10, 1}); + experimental::framework_graph g{driver_for_test(gen)}; g.provide("provide_num", give_me_nums, concurrency::unlimited) .output_product(product_query{.creator = "input", .layer = "event", .suffix = "num"}); for (auto const& [name, b, e] : configs) { @@ -199,7 +189,9 @@ TEST_CASE("Three predicates in parallel", "[filtering]") TEST_CASE("Two predicates in parallel (each with multiple arguments)", "[filtering]") { - experimental::framework_graph g{source{10u}}; + experimental::layer_generator gen; + gen.add_layer("event", {"job", 10, 1}); + experimental::framework_graph g{driver_for_test(gen)}; g.provide("provide_num", give_me_nums, concurrency::unlimited) .output_product(product_query{.creator = "input", .layer = "event", .suffix = "num"}); g.provide("provide_other_num", give_me_other_nums, concurrency::unlimited) diff --git a/test/fixed_hierarchy_test.cpp b/test/fixed_hierarchy_test.cpp new file mode 100644 index 000000000..4f397e0c4 --- /dev/null +++ b/test/fixed_hierarchy_test.cpp @@ -0,0 +1,93 @@ +#include "phlex/model/data_cell_index.hpp" +#include "phlex/model/fixed_hierarchy.hpp" + +#include "catch2/catch_test_macros.hpp" +#include "catch2/matchers/catch_matchers_string.hpp" + +using namespace phlex; +using namespace phlex::experimental; +using namespace Catch::Matchers; + +// Builds indices for the hierarchy from the issue description: +// +// run +// │ +// ├ subrun +// │ │ +// │ └ spill +// │ +// └ calibration_period +// +// Specified as: {{"run", "subrun", "spill"}, {"run", "calibration_period"}} + +namespace { + auto make_hierarchy() + { + return fixed_hierarchy{{"run", "subrun", "spill"}, {"run", "calibration_period"}}; + } +} + +TEST_CASE("Ill-formed paths result in an exception", "[fixed_hierarchy]") +{ + CHECK_THROWS_WITH(fixed_hierarchy{{}}, ContainsSubstring("Layer paths cannot be empty")); + CHECK_THROWS_WITH((fixed_hierarchy{{"job", "run", "job"}}), + ContainsSubstring("Layer paths may only contain 'job' as the first element")); +} + +TEST_CASE("Default-constructed fixed_hierarchy accepts any index", "[fixed_hierarchy]") +{ + fixed_hierarchy const h; + auto const job = data_cell_index::job(); + CHECK_NOTHROW(h.validate(job)); + CHECK_NOTHROW(h.validate(job->make_child("run", 0))); + CHECK_NOTHROW(h.validate(job->make_child("unrelated", 0))); +} + +TEST_CASE("Fixed hierarchy accepts valid layer indices", "[fixed_hierarchy]") +{ + auto const h = make_hierarchy(); + auto const job = data_cell_index::job(); + auto const run = job->make_child("run", 0); + auto const subrun = run->make_child("subrun", 0); + + CHECK_NOTHROW(h.validate(job)); + CHECK_NOTHROW(h.validate(run)); + CHECK_NOTHROW(h.validate(subrun)); + CHECK_NOTHROW(h.validate(subrun->make_child("spill", 0))); + CHECK_NOTHROW(h.validate(run->make_child("calibration_period", 0))); +} + +TEST_CASE("Fixed hierarchy rejects indices not present in the hierarchy", "[fixed_hierarchy]") +{ + auto const h = make_hierarchy(); + auto const job = data_cell_index::job(); + + // Layer not declared in the hierarchy at all + CHECK_THROWS_WITH(h.validate(job->make_child("unknown", 0)), + ContainsSubstring("Layer /job/unknown is not part of the fixed hierarchy")); + + // "subrun" exists but only under "run", not directly under "job" + CHECK_THROWS_WITH(h.validate(job->make_child("subrun", 0)), + ContainsSubstring("Layer /job/subrun is not part of the fixed hierarchy")); +} + +TEST_CASE("Paths with and without 'job' prefix produce the same hierarchy", "[fixed_hierarchy]") +{ + fixed_hierarchy const without_prefix{{"run", "subrun"}, {"run", "calibration_period"}}; + fixed_hierarchy const with_prefix{{"job", "run", "subrun"}, {"job", "run", "calibration_period"}}; + + auto const job = data_cell_index::job(); + auto const run = job->make_child("run", 0); + auto const subrun = run->make_child("subrun", 0); + auto const cal_period = run->make_child("calibration_period", 0); + auto const unknown = job->make_child("unknown", 0); + + for (auto const* h : {&without_prefix, &with_prefix}) { + CHECK_NOTHROW(h->validate(job)); + CHECK_NOTHROW(h->validate(run)); + CHECK_NOTHROW(h->validate(subrun)); + CHECK_NOTHROW(h->validate(cal_period)); + CHECK_THROWS_WITH(h->validate(unknown), + ContainsSubstring("Layer /job/unknown is not part of the fixed hierarchy")); + } +} diff --git a/test/framework_graph.cpp b/test/framework_graph.cpp index f7b57c462..6aa6a8168 100644 --- a/test/framework_graph.cpp +++ b/test/framework_graph.cpp @@ -10,13 +10,14 @@ using namespace phlex; TEST_CASE("Catch STL exceptions", "[graph]") { - experimental::framework_graph g{[](framework_driver&) { throw std::runtime_error("STL error"); }}; + experimental::framework_graph g{ + [](experimental::framework_driver&) { throw std::runtime_error("STL error"); }}; CHECK_THROWS_AS(g.execute(), std::exception); } TEST_CASE("Catch other exceptions", "[graph]") { - experimental::framework_graph g{[](framework_driver&) { throw 2.5; }}; + experimental::framework_graph g{[](experimental::framework_driver&) { throw 2.5; }}; CHECK_THROWS_AS(g.execute(), double); } diff --git a/test/function_registration.cpp b/test/function_registration.cpp index 93f038bdc..dfa648e7a 100644 --- a/test/function_registration.cpp +++ b/test/function_registration.cpp @@ -61,7 +61,7 @@ TEST_CASE("Call non-framework functions", "[programming model]") std::array const product_suffixes = {"onumber"s, "otemperature"s, "oname"s}; std::array const result{"result"s}; - experimental::framework_graph g{data_cell_index::base_ptr()}; + experimental::framework_graph g; // Register providers g.provide("provide_number", provide_number, concurrency::unlimited) diff --git a/test/multiple_function_registration.cpp b/test/multiple_function_registration.cpp index a2e2de2bb..2293ba96e 100644 --- a/test/multiple_function_registration.cpp +++ b/test/multiple_function_registration.cpp @@ -41,7 +41,7 @@ namespace { TEST_CASE("Call multiple functions", "[programming model]") { - experimental::framework_graph g{data_cell_index::base_ptr()}; + experimental::framework_graph g; g.provide("provide_numbers", [](data_cell_index const&) -> std::vector { return {0, 1, 2, 3, 4}; }) diff --git a/test/product_handle.cpp b/test/product_handle.cpp index 11224a3a7..1e976b6b6 100644 --- a/test/product_handle.cpp +++ b/test/product_handle.cpp @@ -53,7 +53,7 @@ TEST_CASE("Handle copies and moves", "[data model]") spec_t two_spec{"two"}; spec_t four_spec{"four"}; - auto job_data_cell = data_cell_index::base_ptr(); + auto job_data_cell = data_cell_index::job(); auto subrun_6_data_cell = job_data_cell->make_child("subrun", 6); handle h2{two, *job_data_cell, two_spec}; @@ -82,12 +82,12 @@ TEST_CASE("Handle comparisons", "[data model]") int const eighteen{18}; spec_t seventeen_spec{"seventeen"}; spec_t eighteen_spec{"eighteen"}; - handle const h17{seventeen, data_cell_index::base(), seventeen_spec}; - handle const h18{eighteen, data_cell_index::base(), eighteen_spec}; + handle const h17{seventeen, *data_cell_index::job(), seventeen_spec}; + handle const h18{eighteen, *data_cell_index::job(), eighteen_spec}; CHECK(h17 == h17); CHECK(h17 != h18); - auto subrun_6_data_cell = data_cell_index::base_ptr()->make_child("subrun", 6); + auto subrun_6_data_cell = data_cell_index::job()->make_child("subrun", 6); handle const h17sr{seventeen, *subrun_6_data_cell, seventeen_spec}; CHECK(*h17 == *h17sr); // Products are the same CHECK(h17.data_cell_index() != h17sr.data_cell_index()); // Data cells are not the same @@ -98,8 +98,8 @@ TEST_CASE("Handle type conversions (run-time checks)", "[data model]") { int const number{3}; spec_t spec{"number"}; - handle const h{number, data_cell_index::base(), spec}; - CHECK(h.data_cell_index() == data_cell_index::base()); + handle const h{number, *data_cell_index::job(), spec}; + CHECK(h.data_cell_index() == *data_cell_index::job()); int const& num_ref = h; int const* num_ptr = h; @@ -109,7 +109,7 @@ TEST_CASE("Handle type conversions (run-time checks)", "[data model]") Composer const composer{"Elgar"}; spec_t composer_spec{"composer"}; - CHECK(handle{composer, data_cell_index::base(), composer_spec}->name == "Elgar"); + CHECK(handle{composer, *data_cell_index::job(), composer_spec}->name == "Elgar"); } TEST_CASE("Retrieve product specification from handle", "[data model]") @@ -117,7 +117,7 @@ TEST_CASE("Retrieve product specification from handle", "[data model]") int const number{3}; spec_t spec{"creator/three"}; - handle const h{number, data_cell_index::base(), spec}; + handle const h{number, *data_cell_index::job(), spec}; CHECK(h.creator().algorithm == "creator"); CHECK(h.suffix() == "three"); CHECK(h.layer() == "job"); diff --git a/test/repeater_node_test.cpp b/test/repeater_node_test.cpp index 86cfde321..6a1afc046 100644 --- a/test/repeater_node_test.cpp +++ b/test/repeater_node_test.cpp @@ -20,7 +20,7 @@ using namespace phlex::experimental; namespace { auto make_run_index(int run_number) { - return phlex::data_cell_index::base_ptr()->make_child("run", run_number); + return phlex::data_cell_index::job()->make_child("run", run_number); } auto make_run_with_product(int run_number, int value) diff --git a/test/type_distinction.cpp b/test/type_distinction.cpp index c5bf4999a..fe60dbff3 100644 --- a/test/type_distinction.cpp +++ b/test/type_distinction.cpp @@ -1,6 +1,8 @@ #include "phlex/core/framework_graph.hpp" +#include "phlex/driver.hpp" #include "phlex/model/data_cell_index.hpp" #include "phlex/model/product_store.hpp" +#include "plugins/layer_generator.hpp" #include "spdlog/spdlog.h" @@ -42,18 +44,10 @@ namespace { TEST_CASE("Distinguish products with same name and different types", "[programming model]") { + experimental::layer_generator gen; + gen.add_layer("event", {"job", 10, 1}); - auto gen = [](auto& driver) { - std::vector numbers{1, 2, 3, 4, 5, 6, 7, 8, 9, 10}; - auto job_index = data_cell_index::base_ptr(); - driver.yield(job_index); - for (int i : numbers) { - auto event_index = job_index->make_child("event", unsigned(i)); - driver.yield(event_index); - } - }; - - experimental::framework_graph g{gen}; + experimental::framework_graph g{driver_for_test(gen)}; // Register providers g.provide("provide_numbers", provide_numbers, concurrency::unlimited) diff --git a/test/yielding_driver.cpp b/test/yielding_driver.cpp index a88d32c73..3953f3b55 100644 --- a/test/yielding_driver.cpp +++ b/test/yielding_driver.cpp @@ -16,7 +16,7 @@ void cells_to_process(experimental::async_driver& d) unsigned int const num_subruns = 2; unsigned int const num_spills = 3; - auto job_id = data_cell_index::base_ptr(); + auto job_id = data_cell_index::job(); d.yield(job_id); for (unsigned int r : std::views::iota(0u, num_runs)) { auto run_id = job_id->make_child("run", r); @@ -34,7 +34,7 @@ void cells_to_process(experimental::async_driver& d) TEST_CASE("Async driver with TBB flow graph", "[async_driver]") { experimental::async_driver drive{cells_to_process}; - std::vector received_ids; + std::vector received_indices; tbb::flow::graph g{}; tbb::flow::input_node source{g, [&drive](tbb::flow_control& fc) -> data_cell_index_ptr { @@ -47,8 +47,8 @@ TEST_CASE("Async driver with TBB flow graph", "[async_driver]") tbb::flow::function_node receiver{ g, tbb::flow::serial, - [&received_ids](data_cell_index_ptr const& set_id) -> tbb::flow::continue_msg { - received_ids.push_back(set_id->to_string()); + [&received_indices](data_cell_index_ptr const& set_id) -> tbb::flow::continue_msg { + received_indices.push_back(set_id->to_string()); return {}; }}; @@ -56,5 +56,5 @@ TEST_CASE("Async driver with TBB flow graph", "[async_driver]") source.activate(); g.wait_for_all(); - CHECK(received_ids.size() == 19); + CHECK(received_indices.size() == 19); }