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.

Sign up for GitHub

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

Jump to bottom

[SGD] Sgd v2 Dataset Integration #17626

Merged
merged 60 commits into from
Oct 12, 2021
Merged

[SGD] Sgd v2 Dataset Integration #17626

merged 60 commits into from
Oct 12, 2021

Conversation

amogkam
Copy link
Contributor

@amogkam amogkam commented Aug 6, 2021 •
edited
Loading

Adds Dataset Integration with Ray SGD v2.

Users can pass in a Ray Dataset or DatasetPipeline into their Trainer. Internally, SGD will split the dataset into num_workers equal size shards, and also provide the actor handles for locality hints. The user can then access the shard for a particular worker in their training function via sgd.get_dataset_shard().

Why are these changes needed?

Related issue number

Checks

  • 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 :(

@amogkam amogkam marked this pull request as draft August 6, 2021 01:19
@amogkam amogkam added the do-not-merge Do not merge this PR! label Aug 6, 2021
ericl
ericl reviewed Oct 9, 2021
View reviewed changes
python/ray/data/dataset.py Outdated Show resolved Hide resolved
@ericl ericl added the @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. label Oct 9, 2021
@ericl ericl self-assigned this Oct 9, 2021
@amogkam amogkam requested a review from scv119 as a code owner October 10, 2021 03:06
clarkzinzow
clarkzinzow reviewed Oct 11, 2021
View reviewed changes
Copy link
Contributor

@clarkzinzow clarkzinzow left a comment

Choose a reason for hiding this comment

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

First pass, awesome docs and examples!

doc/source/data/dataset-pipeline.rst Outdated Show resolved Hide resolved
doc/source/data/dataset-pipeline.rst Show resolved Hide resolved
doc/source/data/dataset-pipeline.rst Show resolved Hide resolved
doc/source/raysgd/v2/user_guide.rst Outdated Show resolved Hide resolved
doc/source/raysgd/v2/user_guide.rst Outdated Show resolved Hide resolved
doc/source/raysgd/v2/user_guide.rst Show resolved Hide resolved
doc/source/raysgd/v2/user_guide.rst Outdated Show resolved Hide resolved
python/ray/data/dataset.py Outdated Show resolved Hide resolved
python/ray/util/sgd/v2/trainer.py Outdated Show resolved Hide resolved
python/ray/util/sgd/v2/trainer.py Outdated Show resolved Hide resolved
@matthewdeng matthewdeng removed the @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. label Oct 12, 2021
@ericl ericl added the @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. label Oct 12, 2021
@amogkam amogkam added tests-ok The tagger certifies test failures are unrelated and assumes personal liability. and removed @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. labels Oct 12, 2021
@amogkam
Copy link
Contributor Author

amogkam commented Oct 12, 2021

Failing test is unrelated, going to merge

@amogkam amogkam merged commit f6f2435 into ray-project:master Oct 12, 2021
@amogkam amogkam deleted the sgd-v2-datasets branch October 12, 2021 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@clarkzinzow clarkzinzow clarkzinzow left review comments

@richardliaw richardliaw richardliaw left review comments

@matthewdeng matthewdeng matthewdeng left review comments

@ericl ericl ericl approved these changes

@scv119 scv119 Awaiting requested review from scv119

Assignees

@ericl ericl

@scv119 scv119

@matthewdeng matthewdeng

@richardliaw richardliaw

@clarkzinzow clarkzinzow

Labels
tests-ok The tagger certifies test failures are unrelated and assumes personal liability.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@amogkam @ericl @scv119 @matthewdeng @richardliaw @clarkzinzow