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] Add rule for ReorderRandomizeBlockOrder #32254

Merged
merged 10 commits into from
Feb 10, 2023

Conversation

amogkam
Copy link
Contributor

@amogkam amogkam commented Feb 7, 2023

Ports over previous rule to move RandomizeBlockOrder to the end of a DAG into the new execution backend as an optimizer rule.

Closes #31894

Why are these changes needed?

Related issue number

Checks

  • I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

Signed-off-by: amogkam <amogkamsetty@yahoo.com>
Signed-off-by: amogkam <amogkamsetty@yahoo.com>
Signed-off-by: amogkam <amogkamsetty@yahoo.com>
Signed-off-by: amogkam <amogkamsetty@yahoo.com>
Signed-off-by: amogkam <amogkamsetty@yahoo.com>
Signed-off-by: amogkam <amogkamsetty@yahoo.com>
Signed-off-by: amogkam <amogkamsetty@yahoo.com>
Signed-off-by: amogkam <amogkamsetty@yahoo.com>
return LogicalPlan(dag=optimized_dag)

def _apply(self, op: LogicalOperator) -> LogicalOperator:
seeds = []
Copy link
Contributor

Choose a reason for hiding this comment

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

as discussed, let's keep a list of RandomizeBlocks operator, instead of seed, to be future-proof when we add more attributes into RandomizeBlocks. Thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

Signed-off-by: amogkam <amogkamsetty@yahoo.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

@amogkam amogkam changed the title [Data] Add rule for RandomizeBlockOrder [Data] Add rule for ReorderRandomizeBlockOrder Feb 10, 2023
@amogkam amogkam merged commit b4ad23a into ray-project:master Feb 10, 2023
@amogkam amogkam deleted the random-block-order branch February 10, 2023 00:28
edoakes pushed a commit to edoakes/ray that referenced this pull request Mar 22, 2023
Ports over previous rule to move RandomizeBlockOrder to the end of a DAG into the new execution backend as an optimizer rule.

Closes ray-project#31894

Signed-off-by: amogkam <amogkamsetty@yahoo.com>
Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
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.

[Datasets] Implement reordering RandomizeBlocksStage as optimizer rule
3 participants