Conversation
Codecov Report❌ Patch coverage is @@ Coverage Diff @@
## main #180 +/- ##
==========================================
+ Coverage 81.00% 81.13% +0.13%
==========================================
Files 114 116 +2
Lines 2037 2083 +46
Branches 328 332 +4
==========================================
+ Hits 1650 1690 +40
- Misses 254 255 +1
- Partials 133 138 +5
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR introduces a provider node mechanism to the Phlex framework, which fundamentally changes how data products are created and injected into the processing graph. Instead of embedding data products directly into product_store objects at the source level, providers generate products on-demand when they are needed, based on the data_cell_index.
Key Changes
- Introduced
declared_providerclass andprovider_nodetemplate for implementing lazy, on-demand data generation - Added
provide()API to framework_graph and glue interfaces for registering provider functions - Modified
edge_makerto automatically wire provider outputs to consumer inputs based on product labels and families - Implemented caching in providers to avoid redundant computation for the same data_cell_index
- Simplified
product_storeby removing parent tracking methods (make_child,make_continuation,parent(), etc.) - Changed
framework_driverfromasync_driver<product_store_ptr>toasync_driver<data_cell_index_ptr>
Reviewed changes
Copilot reviewed 70 out of 70 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| phlex/core/declared_provider.{hpp,cpp} | New provider node implementation with caching and flush handling |
| phlex/core/registration_api.hpp | Added provider_api class for fluent registration interface |
| phlex/core/edge_maker.{hpp,cpp} | Added make_provider_edges() to wire providers to consumers automatically |
| phlex/core/multiplexer.{hpp,cpp} | Modified to route messages to provider nodes based on data layer |
| phlex/core/node_catalog.{hpp,cpp} | Added provider tracking and execution counts |
| phlex/core/concepts.hpp | Added is_provider_like concept for provider function validation |
| phlex/core/framework_graph.{hpp,cpp} | Integrated provider API and changed driver to use data_cell_index |
| phlex/core/glue.hpp | Added provide() method for provider registration |
| phlex/model/product_store.{hpp,cpp} | Simplified by removing parent relationship methods |
| test/* | Updated all tests to use provider pattern instead of direct product injection |
| test/demo-giantdata/* | Removed logging infrastructure, simplified data flow with providers |
| test/benchmarks/* | Added benchmarks_provider module and updated configurations |
|
@copilot What would it take to improve the test coverage of C++ code in this change to 100%? According to Codecov, 21 lines have partial or missing coverage. |
|
@greenc-FNAL I've opened a new pull request, #182, to work on those changes. Once the pull request is ready, I'll request review from you. |
Implement give_me_vertices provider algorithm that returns toy::VertexCollection struct instead of fundamental type. Wire stub provider into framework_graph with unlimited concurrency. Update VertexCollection::data to use std::size_t.
- Add provide() member to glue class for provider registration - Document glue class template with comprehensive description of its role in flow graph registration - Document make_glue() helper explaining void_tag behavior and state management - Update provider_test to use provide() instead of provider() for consistency with other node types
Introduce declared_provider class and provider_node template for handling data input into the graph. Providers accept level_id and generate products. Add provider registration to node_catalog and glue interface. Register providers using input_family syntax for consistent product association. Update provider_test to use level_id reference and specified_label syntax for provider registration.
Add provider_node template class that creates data products on-demand from level IDs. Providers are registered via the provide() API and wired into the graph by edge_maker, which matches provider outputs to consumer inputs based on product labels and families. Modify multiplexer to route messages to provider nodes based on level information. Update node_catalog to track provider execution counts. Add provider_test.cpp demonstrating provider registration and execution with a toy VertexCollection example. This development is not yet complete. All the libraries compile and link, and all the tests compile and link. But most tests that use a graph fail, because they have not yet been updated to include the providers needed to work with this version of the infrastructure.
Also includes slight adjustment of product_store constructor arguments
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
pcanal
left a comment
pcanal
left a comment
There was a problem hiding this comment.
From random sampling, the code looks self consistent, so LGTM.
Unfortunately, PR #182 will not address the code coverage problems. Those will have to be solved gradually as we move forward. |
This PR introduces the first prototype of the provider nodes to the Phlex framework.
Key changes include:
declared_providerclass andprovider_nodetemplate for implementing provider functionalityprovide()API in the glue interface for registering providers with the frameworkedge_makerto automatically wire provider outputs to consumer inputs based on product labels and familiesmultiplexerto route level-based messages to provider nodes during executionnode_catalogto track provider execution counts