Skip to content
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] New executor [10/n]--- Plumbing for locality_with_output and setting execution options #31908

Merged
merged 95 commits into from
Jan 26, 2023

Conversation

ericl
Copy link
Contributor

@ericl ericl commented Jan 24, 2023

Why are these changes needed?

  1. Support locality_with_output.
  2. Support setting execution options via DatasetContext for now (and a TODO to add it to the public API).
  3. Add unit test for configuration plumbing.

ericl added 30 commits January 10, 2023 15:44
Signed-off-by: Eric Liang <ekhliang@gmail.com>
Signed-off-by: Eric Liang <ekhliang@gmail.com>
Signed-off-by: Eric Liang <ekhliang@gmail.com>
Signed-off-by: Eric Liang <ekhliang@gmail.com>
Signed-off-by: Eric Liang <ekhliang@gmail.com>
Signed-off-by: Eric Liang <ekhliang@gmail.com>
Signed-off-by: Eric Liang <ekhliang@gmail.com>
Signed-off-by: Eric Liang <ekhliang@gmail.com>
Signed-off-by: Eric Liang <ekhliang@gmail.com>
Signed-off-by: Eric Liang <ekhliang@gmail.com>
Signed-off-by: Eric Liang <ekhliang@gmail.com>
Signed-off-by: Eric Liang <ekhliang@gmail.com>
Signed-off-by: Eric Liang <ekhliang@gmail.com>
ericl added 7 commits January 23, 2023 12:24
Signed-off-by: Eric Liang <ekhliang@gmail.com>
Signed-off-by: Eric Liang <ekhliang@gmail.com>
Signed-off-by: Eric Liang <ekhliang@gmail.com>
ericl added 4 commits January 24, 2023 14:50
Signed-off-by: Eric Liang <ekhliang@gmail.com>
Signed-off-by: Eric Liang <ekhliang@gmail.com>
Signed-off-by: Eric Liang <ekhliang@gmail.com>
Copy link
Contributor

@c21 c21 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM w/ minor comment.

from ray.data._internal.execution.legacy_compat import (
execute_to_legacy_block_iterator,
)

executor = StreamingExecutor(ExecutionOptions())
executor = StreamingExecutor(copy.deepcopy(ctx.execution_options))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

given execution_options could be muted in StreamingExecutor/BulkExecutor, shall we just provide a method in DatasetContext.get_execution_options() to just return a deep copy of ctx.execution_options?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missed this comment--- I'll file a followup.

@@ -171,6 +175,8 @@ def __init__(
self.enable_auto_log_stats = enable_auto_log_stats
self.trace_allocations = trace_allocations
self.optimizer_enabled = optimizer_enabled
# TODO: expose execution options in Dataset public APIs.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be better to create a tracking issue

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added to #31797

@ericl ericl merged commit c8f11ce into ray-project:master Jan 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants