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.
Related Issues
Proposed Changes:
Remove the cycle check completely in pipeline. By ensuring that
find_next_runnable_node
only returns nodes that can actually be run, we don't need the check: Everything in therun_queue
will be run and once its empty, we are done.Instead of previously splitting the graph up into acyclic subgraphs, we maintain a
run_queue
which only contains nodes that can be run as is. Once we execute a node from therun_queue
, we remove it and add all its runnable successors. That way, cycles don't need any special handling. If a backedge supplies all necessary inputs to a node at the top of a loop, it will also be added. This restarts the loop.How did you test it?
Unit tests and manual verification in my project.
Notes for the reviewer
Note that similar concerns about non-determinism from #8677 apply. For example, nodes with only default input may run in any order. #8680 addresses that specific issue. Nodes within the run_queue are still run in a non-deterministic order but they should be independent anyway.
I might also have found an error in a test: Instead of returning a document_builder (which expects but doesn't get any inputs), we now expect the document_joiner (which is variadic and thus okay with not getting any inputs).
Checklist
fix:
,feat:
,build:
,chore:
,ci:
,docs:
,style:
,refactor:
,perf:
,test:
and added!
in case the PR includes breaking changes.