-
Notifications
You must be signed in to change notification settings - Fork 6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[data] Fix wrong output order of streaming_split #36919
[data] Fix wrong output order of streaming_split #36919
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah... great catch!
@@ -103,6 +103,17 @@ def run(self): | |||
c1.start() | |||
c0.join() | |||
c1.join() | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could also add a unit test to test_operators.py for OutputSplitter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added
Signed-off-by: Hao Chen <chenh1024@gmail.com>
9acdfe8
to
8a04b2f
Compare
The output order of OutputSplitter should be FIFO, instead of LIFO. This bug also makes streaming_split's order non-deterministic, because it depends on when the OutputSplitter's outputs are taken. --------- Signed-off-by: Hao Chen <chenh1024@gmail.com> Signed-off-by: e428265 <arvind.chandramouli@lmco.com>
Why are these changes needed?
The output order of
OutputSplitter
should be FIFO, instead of LIFO. This bug also makes streaming_split's order non-deterministic, because it depends on when the OutputSplitter's outputs are taken.Related issue number
#35053
Checks
git commit -s
) in this PR.scripts/format.sh
to lint the changes in this PR.method in Tune, I've added it in
doc/source/tune/api/
under thecorresponding
.rst
file.