🏆 Live Soccer TV: ray project/ray/pull/

woshiyyya · 2023-03-09T01:51:11Z

Why are these changes needed?

There will be a list of PRs for PyTorch Lightning Integration. This is the first one.

LightningTrainer + Test
Logging and Checkpointing + Test
Predictor + Batch Prediction + Test
Integrate with Tune + Test

Content for this PR:

LightningTrainer Interface design
Support DDP Training
Examples and docstrings

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: woshiyyya <xiaoyunxuan1998@gmail.com>

amogkam · 2023-03-11T05:09:01Z

great work @woshiyyya!

justinvyu

Looks very good!

Co-authored-by: Amog Kamsetty <amogkam@users.noreply.github.com> Co-authored-by: Justin Yu <justinvyu@anyscale.com> Signed-off-by: Yunxuan Xiao <xiaoyunxuan1998@gmail.com>

Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>

…e_trainer

gjoliver

looks quite solid. only a few minor comments.

gjoliver · 2023-03-14T07:30:48Z

+        def _train_dataloader() -> TRAIN_DATALOADERS:
+            assert train_dataset
+            ds = RayIterableDataset(train_dataset, dataset_iter_config)
+            return DataLoader(ds, batch_size=1, collate_fn=lambda x: x[0])


why would iter_torch_batches() add an extra dim of 1 here???

gjoliver · 2023-03-14T07:33:15Z

+    ``pytorch_lightning.Trainer.fit``.
+
+    Example:
+        .. code-block:: python


ok. add a TODO?

Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>

…e_trainer

gjoliver

@amogkam let us know if you have any more comments. we are gonna merge this now, which is just a start.

amogkam

Lgtm overall

now that the api is finalized, can we test the docstring?
Can we add a multi-node, multi-gpu release test as the next followup PR? We want to validate that multi node gpu training is working as soon as possible. We can just rewrite the existing ray Lightning release test.

amogkam · 2023-03-15T03:54:34Z

+    """Configuration Class to pass into LightningTrainer.
+
+    Example:
+        .. code-block:: python


Can we test this code snippet now that the api has been finalized?

Just replace code-block with test-code, and I think that should work

Sure thing. My plan is to create a separate PR for release tests. I've tested on workspace that multi-node multi-machine training works. Also I wrote some ci tests with single-node multi-gpu setting which also work. I'll post a PR tomorrow to address these testing issues.

For the code in docstring, I'll test it as well, together with the release test PR.

…<Part 1>. (ray-project#33161) * init Lightning Trainer with ci tests Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com> Signed-off-by: Jack He <jackhe2345@gmail.com>

…<Part 1>. (ray-project#33161) * init Lightning Trainer with ci tests Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com> Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>

…<Part 1>. (ray-project#33161) * init Lightning Trainer with ci tests Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>

…<Part 1>. (ray-project#33161) * init Lightning Trainer with ci tests Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com> Signed-off-by: elliottower <elliot@elliottower.com>

…<Part 1>. (ray-project#33161) * init Lightning Trainer with ci tests Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com> Signed-off-by: Jack He <jackhe2345@gmail.com>

init Lightning Trainer with ci tests

4c8edc1

Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>

woshiyyya assigned justinvyu, amogkam and Yard1 Mar 9, 2023

woshiyyya requested review from Yard1, amogkam, gjoliver, justinvyu and matthewdeng March 9, 2023 02:01

woshiyyya assigned matthewdeng and gjoliver Mar 9, 2023

add test for trainer with categorical ray dataset

e0fd5d9

Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>

woshiyyya changed the title ~~[AIR] Add LightningTrainer to support Pytorch Lightning DDP training.~~ [AIR] <Part 1> Add LightningTrainer to support Pytorch Lightning DDP training. Mar 9, 2023

woshiyyya added 2 commits March 8, 2023 19:18

fix lightiningEnvironment import error

ebea347

Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>

fix test_lightning_trainer not found error

01f9461

Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>

woshiyyya force-pushed the air/lightning_base_trainer branch from 14e3ee3 to 01f9461 Compare March 9, 2023 06:18

woshiyyya marked this pull request as ready for review March 9, 2023 17:08

format code

22725b6

Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>

woshiyyya unassigned matthewdeng Mar 9, 2023

check linter

b3ab873

Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>

woshiyyya added air train Ray Train Related Issue labels Mar 9, 2023

woshiyyya added 2 commits March 9, 2023 17:17

fix linting issues

a9d8ab6

Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>

Change lightning_config to dict type to aligh with Ray Tune

a75fec6

Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>

amogkam reviewed Mar 11, 2023

View reviewed changes

justinvyu reviewed Mar 12, 2023

View reviewed changes

gjoliver reviewed Mar 12, 2023

View reviewed changes

woshiyyya and others added 2 commits March 12, 2023 19:01

Apply suggestions from code review

895ab43

Co-authored-by: Amog Kamsetty <amogkam@users.noreply.github.com> Co-authored-by: Justin Yu <justinvyu@anyscale.com> Signed-off-by: Yunxuan Xiao <xiaoyunxuan1998@gmail.com>

replace LightningConfigBuilder setter with semantic meanings

f76f48d

Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>

woshiyyya added 2 commits March 13, 2023 01:44

add fixture and builder_tests

c1448db

Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>

add example code for config builder, add api index

66ab606

Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>

woshiyyya requested review from a team, krfricke, maxpumperla, richardliaw and xwjiang2010 as code owners March 13, 2023 18:52

woshiyyya added 4 commits March 13, 2023 13:58

fix lint, change test tag to large

2842b14

Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>

remove resume from ckpt for next PR

38cd101

Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>

rename trainer.rst to avoid cross reference

5210cb7

Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>

Merge remote-tracking branch 'upstream/master' into air/lightning_bas…

300ba71

…e_trainer

gjoliver reviewed Mar 14, 2023

View reviewed changes

woshiyyya added 3 commits March 14, 2023 12:02

address comments

1935473

Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>

fix document issue

55a7b32

Signed-off-by: woshiyyya <xiaoyunxuan1998@gmail.com>

Merge remote-tracking branch 'upstream/master' into air/lightning_bas…

6598894

…e_trainer

woshiyyya requested a review from gjoliver March 15, 2023 02:22

gjoliver approved these changes Mar 15, 2023

View reviewed changes

matthewdeng approved these changes Mar 15, 2023

View reviewed changes

amogkam reviewed Mar 15, 2023

View reviewed changes

richardliaw approved these changes Mar 15, 2023

View reviewed changes

gjoliver merged commit 0574620 into ray-project:master Mar 15, 2023

Uh oh!

Conversation

woshiyyya commented Mar 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why are these changes needed?

Related issue number

Checks

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

amogkam commented Mar 11, 2023

Uh oh!

justinvyu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gjoliver left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

gjoliver Mar 14, 2023

Choose a reason for hiding this comment

Uh oh!

gjoliver Mar 14, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gjoliver left a comment

Choose a reason for hiding this comment

Uh oh!

amogkam left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amogkam Mar 15, 2023

Choose a reason for hiding this comment

Uh oh!

amogkam Mar 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

woshiyyya Mar 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

woshiyyya Mar 15, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

woshiyyya commented Mar 9, 2023 •

edited

Loading

amogkam left a comment •

edited

Loading

amogkam Mar 15, 2023 •

edited

Loading

woshiyyya Mar 15, 2023 •

edited

Loading