-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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
Streaming executor fixes #4 #32882
Streaming executor fixes #4 #32882
Conversation
Signed-off-by: jianoaix <[email protected]>
@@ -127,7 +127,11 @@ def test_new_execution_backend_invocation(ray_start_10_cpus_shared): | |||
DatasetContext.get_current().new_execution_backend = True |
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.
This test is called test_bulk_executor
so shall we just force bulk execution mode instead?
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.
Makes sense!
Failing tests seem not related. |
ds = ds1.zip(ds2).fully_executed() | ||
num_blocks = ds._plan._snapshot_blocks.executed_num_blocks() | ||
assert ds.take() == [{str(i): i for i in range(num_cols1 + num_cols2)}] * n | ||
if should_invert: | ||
assert ds.num_blocks() == num_blocks2 | ||
assert num_blocks == num_blocks2 | ||
else: | ||
assert ds.num_blocks() == num_blocks1 | ||
assert num_blocks == num_blocks1 |
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.
curious as #32860 is merged, would the change here is still needed?
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.
Have you tested that PR with streaming enabled for this test? Also note this is to handle issue introduced in #32132.
Test failures |
Signed-off-by: Jack He <[email protected]>
Signed-off-by: Edward Oakes <[email protected]>
Signed-off-by: elliottower <[email protected]>
Signed-off-by: Jack He <[email protected]>
Why are these changes needed?
The streaming doesn't preserve order by default, so the results comparison will need to use set instead of list.
#32132
Related issue number
Checks
git commit -s
) in this PR.scripts/format.sh
to lint the changes in this PR.