Support multi-layer algorithms that receive a data product from an unfold#571
Open
knoepfel wants to merge 4 commits intoFramework-R-D:mainfrom
Open
Support multi-layer algorithms that receive a data product from an unfold#571knoepfel wants to merge 4 commits intoFramework-R-D:mainfrom
knoepfel wants to merge 4 commits intoFramework-R-D:mainfrom
Conversation
381d7c1 to
fa25cb1
Compare
Codecov Report❌ Patch coverage is @@ Coverage Diff @@
## main #571 +/- ##
==========================================
+ Coverage 85.39% 85.97% +0.57%
==========================================
Files 145 149 +4
Lines 3711 3885 +174
Branches 646 688 +42
==========================================
+ Hits 3169 3340 +171
Misses 329 329
- Partials 213 216 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 4 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
8bf5ae1 to
b22d4a4
Compare
…uct from an unfold
3f63df8 to
7ef44fb
Compare
7ef44fb to
2312886
Compare
2312886 to
67e4689
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR fixes a bug where transforms operating on a hierarchy layer downstream of an unfold could not receive data products produced by that unfold. The root cause was that the
index_routerhad no mechanism to learn — from the unfold itself — how many children it had generated, so it could not emit the correct flush token at the right time.Solution
The fix introduces two new components and refactors the flush/routing pipeline.
New:
data_cell_tracker(phlex/model/)Tracks the sequence of incoming
data_cell_indexvalues from the driver and determines which flush tokens are ready to be emitted when a new index arrives or the job ends. This logic was previously implicit in the source/index-router interaction.New:
flush_counter(phlex/core/)Per-index counter that tracks how many children have been processed across each child layer. Once the expected count (received from the unfold's flush result) is satisfied,
flush_counterfires a callback to emit the flush token for that index.declared_unfoldNow uses a third TBB output port (
index_flush) carrying the child counts alongside the existing message and index-message ports.index_routerGains a
flush_receiverinput and anestablish_layers()initializer that records which layers are produced by unfolds vs. consumed as inputs.route()now accepts the closeout flushes fromdata_cell_trackerrather than computing them internally, anddrain()likewise receives the remaining flushes at job end.framework_graphindex_receiver_node that decouples closeout flush emission from the source node.data_cell_tracker(cell_tracker_) and feeds its output intoindex_router_.route().index_router_.establish_layers()beforefinalize(), using layer metadata gathered from the declared unfolds.flush_sender()toindex_router_.flush_receiver().edge_makerRefactored to return
(provider_input_ports, multilayer_join_index_ports)instead of directly callingindex_router_.finalize(), allowingframework_graphto callestablish_layers()first.Tests
test/data_cell_tracker_test.cpp— unit tests fordata_cell_trackerin isolation.test/flush_counter_propagation_test.cpp— unit-level tests for flush counter propagation using a standaloneindex_counternode, verifying that committed child counts accumulate correctly through nested unfolds without involving the fullframework_graphmachinery.test/fold_duplicate_layer_name_test.cpp— re-enables the previously disableddifferent_hierarchiestest, now covering the case where two layers share the same name in a fold scenario.test/unfold.cpp— additional unfold scenarios exercising multi-layer product consumption.Files changed
phlex/model/data_cell_tracker.hpp,phlex/model/data_cell_tracker.cppphlex/core/flush_counter.hpp,phlex/core/flush_counter.cppphlex/core/index_router.hpp,phlex/core/index_router.cpp,phlex/core/framework_graph.hpp,phlex/core/framework_graph.cpp,phlex/core/declared_unfold.hpp,phlex/core/edge_maker.hpp,phlex/model/fixed_hierarchy.hpp,phlex/model/fixed_hierarchy.cpptest/data_cell_tracker_test.cpp,test/flush_counter_propagation_test.cpp,test/fold_duplicate_layer_name_test.cpptest/unfold.cppSummary largely courtesy of Claude Sonnet 4.6
Resolves #359