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

Standalone Lite: Update LightningLite #14726

Merged
merged 23 commits into from
Sep 16, 2022
Merged

Standalone Lite: Update LightningLite #14726

merged 23 commits into from
Sep 16, 2022

Conversation

awaelchli
Copy link
Contributor

@awaelchli awaelchli commented Sep 15, 2022 •
edited by github-actions bot
Loading

What does this PR do?

Updates the LightningLite class to make use of the newly added standalone connector, strategies, and other APIs.
Also contains minor fixes and todo resolutions.

In the next PR, the class and wrappers will be moved to the standalone package.
In another PR, we will add a subset of integration tests.

Before submitting

  • Was this discussed/approved via a GitHub issue? (not for typos and docs)
  • Did you read the contributor guideline, Pull Request section?
  • Did you make sure your PR does only one thing, instead of bundling different changes together?
  • Did you make sure to update the documentation with your changes? (if necessary)
  • Did you write any new necessary tests? (not for typos and docs)
  • Did you verify new and existing tests pass locally with your changes?
  • Did you update the CHANGELOG? (not for typos, docs, test updates, or internal minor changes/refactorings)

PR review

Anyone in the community is free to review the PR once the tests have passed.
Before you start reviewing make sure you have read Review guidelines. In short, see the following bullet-list:

  • Is this pull request ready for review? (if not, please submit in draft mode)
  • Check that all items from Before submitting are resolved
  • Make sure the title is self-explanatory and the description concisely explains the PR
  • Add labels and milestones (and optionally projects) to the PR so it can be classified

Did you have fun?

I made sure I had fun coding 🙃

cc @Borda @tchaton @rohitgr7 @justusschock @awaelchli @carmocca @akihironitta

@github-actions github-actions bot added the pl Generic label for PyTorch Lightning package label Sep 15, 2022
@awaelchli awaelchli changed the base branch from master to lite/accelerator-connector September 15, 2022 13:46
@awaelchli awaelchli changed the base branch from lite/accelerator-connector to master September 15, 2022 13:46
@awaelchli awaelchli added refactor fabric lightning.fabric.Fabric priority: 0 High priority task labels Sep 15, 2022
@awaelchli awaelchli added this to the pl:1.8 milestone Sep 15, 2022
awaelchli
awaelchli commented Sep 15, 2022
View reviewed changes
src/lightning_lite/lite.py Show resolved Hide resolved
src/lightning_lite/strategies/ddp_spawn.py Show resolved Hide resolved
src/lightning_lite/utilities/distributed.py Show resolved Hide resolved
src/lightning_lite/strategies/ddp_spawn.py Show resolved Hide resolved
src/pytorch_lightning/lite/lite.py Outdated Show resolved Hide resolved
src/pytorch_lightning/lite/lite.py Show resolved Hide resolved
src/pytorch_lightning/overrides/distributed.py Show resolved Hide resolved
tests/tests_lite/test_lite.py Show resolved Hide resolved
tests/tests_pytorch/lite/test_lite.py Show resolved Hide resolved
@awaelchli awaelchli marked this pull request as ready for review September 15, 2022 22:31
@awaelchli awaelchli self-assigned this Sep 15, 2022
@awaelchli awaelchli mentioned this pull request Sep 16, 2022
Merged
11 tasks
carmocca
carmocca approved these changes Sep 16, 2022
View reviewed changes
Copy link
Contributor

@carmocca carmocca left a comment

Choose a reason for hiding this comment

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

🚀

src/lightning_lite/connector.py Show resolved Hide resolved
src/pytorch_lightning/lite/wrappers.py Outdated Show resolved Hide resolved
@mergify mergify bot added the ready PRs ready to be merged label Sep 16, 2022
@mergify mergify bot added has conflicts and removed ready PRs ready to be merged labels Sep 16, 2022
tchaton
tchaton approved these changes Sep 16, 2022
View reviewed changes
Copy link
Contributor

@tchaton tchaton left a comment

Choose a reason for hiding this comment

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

LGTM !

src/lightning_lite/connector.py Show resolved Hide resolved
src/pytorch_lightning/lite/lite.py Show resolved Hide resolved
@awaelchli awaelchli requested a review from otaj as a code owner September 16, 2022 16:35
@mergify mergify bot added ready PRs ready to be merged and removed has conflicts ready PRs ready to be merged labels Sep 16, 2022
@awaelchli awaelchli enabled auto-merge (squash) September 16, 2022 16:44
@awaelchli awaelchli merged commit 47f0d33 into master Sep 16, 2022
@awaelchli awaelchli deleted the lite/integration branch September 16, 2022 17:25
rohitgr7 pushed a commit that referenced this pull request Sep 24, 2022
@awaelchli @rohitgr7
Standalone Lite: Update LightningLite (#14726)
b9a11af
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@carmocca carmocca carmocca approved these changes

@justusschock justusschock justusschock approved these changes

@tchaton tchaton tchaton approved these changes

@Borda Borda Awaiting requested review from Borda Borda is a code owner

@williamFalcon williamFalcon Awaiting requested review from williamFalcon

@kaushikb11 kaushikb11 Awaiting requested review from kaushikb11

@rohitgr7 rohitgr7 Awaiting requested review from rohitgr7

@otaj otaj Awaiting requested review from otaj

Assignees

@awaelchli awaelchli

Labels
fabric lightning.fabric.Fabric pl Generic label for PyTorch Lightning package priority: 0 High priority task ready PRs ready to be merged refactor
Projects
No open projects
Frameworks Planning
Status: Done
Milestone
v1.8
Development

Successfully merging this pull request may close these issues.
4 participants
@awaelchli @carmocca @tchaton @justusschock