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] Refactor all to all op implementations into a separate file #26585

Merged
merged 5 commits into from
Jul 16, 2022

Conversation

ericl
Copy link
Contributor

@ericl ericl commented Jul 14, 2022

Why are these changes needed?

Pull these op implementations into separate classes for readability. This change also simplifies the implementation of #25708

Signed-off-by: Eric Liang <[email protected]>
Signed-off-by: Eric Liang <[email protected]>
@@ -194,6 +194,8 @@ def schema(
Returns:
The schema of the output dataset.
"""
from ray.data._internal.stage_impl import RandomizeBlocksStage
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Circular import problems...

from ray.data import Dataset


class RepartitionStage(AllToAllStage):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

All code moved verbatim except when commented.

context = DatasetContext.get_current()
if context.use_push_based_shuffle:
if output_num_blocks is not None:
raise NotImplementedError(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@stephanie-wang there was a bug where we ignored the num_blocks parameter for random_shuffle, FYI. It appears to crash push-based shuffle, so I disabled it for now.

Signed-off-by: Eric Liang <[email protected]>
Signed-off-by: Eric Liang <[email protected]>
@ericl ericl added the tests-ok The tagger certifies test failures are unrelated and assumes personal liability. label Jul 15, 2022

if TYPE_CHECKING:
from ray.data import Dataset

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice refactoring. Shall we also pull out the one-to-one stages?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was going to do that later (since OneToOne stages are basically all the same, having them as explicit different classes is not that useful for optimization purposes).

@ericl ericl merged commit cf980c3 into ray-project:master Jul 16, 2022
truelegion47 pushed a commit to truelegion47/ray that referenced this pull request Jul 16, 2022
* master: (35 commits)
  [data] Refactor all to all op implementations into a separate file (ray-project#26585)
  [Datasets] Explicitly define Dataset-like APIs in DatasetPipeline class (ray-project#26394)
  [Serve][Part2] Migrate the tests to use deployment graph api (ray-project#26507)
  [Serve] Default to EveryNode when starting Serve from REST API (ray-project#26588)
  Revert "[KubeRay][Autoscaler][Core] Add a flag to disable ray status version check (ray-project#26584)" (ray-project#26597)
  [air] Add initial benchmark section (ray-project#26608)
  [Workflow] Remove workflow execution module (ray-project#26504)
  [air] Add xgboost release test for silver tier(10-node case). (ray-project#26460)
  Revert "Revert "[serve] Use soft constraint for pinning controller on head node (ray-project#25091)" (ray-project#25857)" (ray-project#25858)
  [RLlib] Fixes MARWIL release tests (ray-project#26586)
  [Datasets] Improve read_xxx experience of HTTP file (ray-project#26454)
  Cleanup ActorContext due to multi actor instances got removed. (ray-project#26497)
  Print newest_ckpt_path when resuming trial. (ray-project#26561)
  Fix test_serialization_error_message for pytest 6.x (ray-project#26591)
  [RLlib] Make DQN update_target use only trainable variables. (ray-project#25226)
  [RLlib] In env check, step only expected agents. (ray-project#26425)
  [RLlib] `restart_failed_sub_environments` now works for MA cases and crashes during `reset()`; +more tests and logging; add eval worker sub-env fault tolerance test. (ray-project#26276)
  [runtime env] plugin refactor[4/n]: remove runtime env protobuf (ray-project#26522)
  Improve streaming read performance for default configuration. (ray-project#26587)
  [Dashboard] Fix test dashboard flaky by catch an expected exception (ray-project#26555)
  ...
truelegion47 pushed a commit to truelegion47/ray that referenced this pull request Jul 16, 2022
xwjiang2010 pushed a commit to xwjiang2010/ray that referenced this pull request Jul 19, 2022
avnishn pushed a commit to smorad/ray that referenced this pull request Jul 20, 2022
klwuibm pushed a commit to yuanchi2807/ray that referenced this pull request Jul 27, 2022
franklsf95 pushed a commit to franklsf95/ray that referenced this pull request Aug 2, 2022
gramhagen pushed a commit to gramhagen/ray that referenced this pull request Aug 15, 2022
Stefan-1313 pushed a commit to Stefan-1313/ray_mod that referenced this pull request Aug 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests-ok The tagger certifies test failures are unrelated and assumes personal liability.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants