Better check for programmatic lightningignore #16080

carmocca · 2022-12-15T19:17:25Z

What does this PR do?

Fixes regression where the LightningTrainerMultiNode component fails to start because the work is re-created several times even after the app has been dispatched.

Avoiding the use of an environment variable fixes the issue. self._backend is not None can act as a proxy for this as suggested by @tchaton

This should get tested by re-enabling the multinode tests

Does your PR introduce any breaking changes? If yes, please list them.

None

cc @Borda

github-actions · 2022-12-15T19:17:58Z

⚡ Required checks status: All passing 🟢

Groups summary

🟢 lightning_app: Tests workflow

Check ID	Status
app-pytest (macOS-11, app, 3.8, latest)	success	✅
app-pytest (macOS-11, app, 3.8, oldest)	success	✅
app-pytest (macOS-11, lightning, 3.9, latest)	success	✅
app-pytest (ubuntu-20.04, app, 3.8, latest)	success	✅
app-pytest (ubuntu-20.04, app, 3.8, oldest)	success	✅
app-pytest (ubuntu-20.04, lightning, 3.9, latest)	success	✅
app-pytest (windows-2022, app, 3.8, latest)	success	✅
app-pytest (windows-2022, app, 3.8, oldest)	success	✅
app-pytest (windows-2022, lightning, 3.8, latest)	success	✅

These checks are required after the changes to src/lightning_app/cli/lightning_cli.py, src/lightning_app/core/flow.py, src/lightning_app/core/work.py, tests/tests_app/components/multi_node/test_trainer.py, tests/tests_app/runners/test_cloud.py.

🟢 lightning_app: Examples

Check ID	Status
app-examples (macOS-11, app, 3.9, latest)	success	✅
app-examples (macOS-11, app, 3.9, oldest)	success	✅
app-examples (macOS-11, lightning, 3.9, latest)	success	✅
app-examples (ubuntu-20.04, app, 3.9, latest)	success	✅
app-examples (ubuntu-20.04, app, 3.9, oldest)	success	✅
app-examples (ubuntu-20.04, lightning, 3.9, latest)	success	✅
app-examples (windows-2022, app, 3.9, latest)	success	✅
app-examples (windows-2022, app, 3.9, oldest)	success	✅
app-examples (windows-2022, lightning, 3.9, latest)	success	✅

These checks are required after the changes to .github/workflows/ci-app-examples.yml, src/lightning_app/cli/lightning_cli.py, src/lightning_app/core/flow.py, src/lightning_app/core/work.py, tests/tests_examples_app/conftest.py, tests/tests_examples_app/local/__init__.py, tests/tests_examples_app/public/__init__.py, tests/tests_examples_app/public/test_multi_node.py, examples/app_multi_node/train_lite.py, examples/app_multi_node/train_lt.py, examples/app_multi_node/train_lt_script.py, examples/app_multi_node/train_pytorch.py, examples/app_multi_node/train_pytorch_spawn.py.

🟢 lightning_app: Azure

Check ID	Status
App.cloud-e2e	success	✅

These checks are required after the changes to src/lightning_app/cli/lightning_cli.py, src/lightning_app/core/flow.py, src/lightning_app/core/work.py, tests/tests_examples_app/conftest.py, tests/tests_examples_app/local/__init__.py, tests/tests_examples_app/public/__init__.py, tests/tests_examples_app/public/test_multi_node.py.

🟢 lightning_app: Docs

Check ID	Status
make-doctest (app)	success	✅
make-html (app)	success	✅

These checks are required after the changes to src/lightning_app/cli/lightning_cli.py, src/lightning_app/core/flow.py, src/lightning_app/core/work.py, docs/source-app/levels/basic/hello_components/pl_multinode.py.

🟢 mypy

Check ID	Status
mypy	success	✅

These checks are required after the changes to src/lightning_app/cli/lightning_cli.py, src/lightning_app/core/flow.py, src/lightning_app/core/work.py.

🟢 install

Check ID	Status
install-pkg (ubuntu-22.04, app, 3.7)	success	✅
install-pkg (ubuntu-22.04, app, 3.10)	success	✅
install-pkg (ubuntu-22.04, lite, 3.7)	success	✅
install-pkg (ubuntu-22.04, lite, 3.10)	success	✅
install-pkg (ubuntu-22.04, pytorch, 3.7)	success	✅
install-pkg (ubuntu-22.04, pytorch, 3.10)	success	✅
install-pkg (ubuntu-22.04, lightning, 3.7)	success	✅
install-pkg (ubuntu-22.04, lightning, 3.10)	success	✅
install-pkg (ubuntu-22.04, notset, 3.7)	success	✅
install-pkg (ubuntu-22.04, notset, 3.10)	success	✅
install-pkg (macOS-12, app, 3.7)	success	✅
install-pkg (macOS-12, app, 3.10)	success	✅
install-pkg (macOS-12, lite, 3.7)	success	✅
install-pkg (macOS-12, lite, 3.10)	success	✅
install-pkg (macOS-12, pytorch, 3.7)	success	✅
install-pkg (macOS-12, pytorch, 3.10)	success	✅
install-pkg (macOS-12, lightning, 3.7)	success	✅
install-pkg (macOS-12, lightning, 3.10)	success	✅
install-pkg (macOS-12, notset, 3.7)	success	✅
install-pkg (macOS-12, notset, 3.10)	success	✅
install-pkg (windows-2022, app, 3.7)	success	✅
install-pkg (windows-2022, app, 3.10)	success	✅
install-pkg (windows-2022, lite, 3.7)	success	✅
install-pkg (windows-2022, lite, 3.10)	success	✅
install-pkg (windows-2022, pytorch, 3.7)	success	✅
install-pkg (windows-2022, pytorch, 3.10)	success	✅
install-pkg (windows-2022, lightning, 3.7)	success	✅
install-pkg (windows-2022, lightning, 3.10)	success	✅
install-pkg (windows-2022, notset, 3.7)	success	✅
install-pkg (windows-2022, notset, 3.10)	success	✅

These checks are required after the changes to src/lightning_app/cli/lightning_cli.py, src/lightning_app/core/flow.py, src/lightning_app/core/work.py.

Thank you for your contribution! 💜

Note
This comment is automatically generated and updates for 60 minutes every 180 seconds. If you have any other questions, contact carmocca for help.

for more information, see https://pre-commit.ci

.github/workflows/ci-app-examples.yml

tchaton

LGTM !

Co-authored-by: Jirka Borovec <[email protected]> (cherry picked from commit b1ce263)

* Add function to remove checkpoint to allow override for extended classes (#16067) (cherry picked from commit 10cc677) * minor fix: indent spaces in comment-out (#16076) (cherry picked from commit 385e5e2) * ci: print existing candidates (#16077) (cherry picked from commit 9e89aed) * [App] Fix bug where previously deleted apps cannot be re-run from the CLI (#16082) (cherry picked from commit 5f7403e) * Better check for programmatic lightningignore (#16080) Co-authored-by: Jirka Borovec <[email protected]> (cherry picked from commit b1ce263) * [App] Removing single quote (#16079) (cherry picked from commit 005b6f2) * version 1.8.5.post0 * skip example test that relies on unreleased lite code The examples use LightningLite syntax without the run method, which is only available in master * fix can't instantiate abstract class [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci fix * skip bagua Co-authored-by: Sean Naren <[email protected]> Co-authored-by: Qiushi Pan <[email protected]> Co-authored-by: Ethan Harris <[email protected]> Co-authored-by: Carlos Mocholí <[email protected]> Co-authored-by: Sherin Thomas <[email protected]> Co-authored-by: awaelchli <[email protected]>

Co-authored-by: Jirka Borovec <[email protected]>

* Remove the deprecated profiler imports (#16059) * Revert "Load app before setting LIGHTNING_DISPATCHED" (#16064) Revert "Load app before setting LIGHTNING_DISPATCHED (#16057)" This reverts commit 8d3339a. * [App] Hot fix: Resolve detection of python debugger (#16068) Co-authored-by: thomas <[email protected]> Co-authored-by: Carlos Mocholí <[email protected]> * Load the app before setting `LIGHTNING_DISPATCHED` (#16071) * fix(cloud): detect and ignore venv (#16056) Co-authored-by: Ethan Harris <[email protected]> * Add function to remove checkpoint to allow override for extended classes (#16067) * Drop FairScale sharded parity tests (#16069) * minor fix: indent spaces in comment-out (#16076) * ci: print existing candidates (#16077) * [App] Fix bug where previously deleted apps cannot be re-run from the CLI (#16082) * Better check for programmatic lightningignore (#16080) Co-authored-by: Jirka Borovec <[email protected]> * [App] Removing single quote (#16079) * [App] PoC: Add support for Request (#16047) * Have checkgroup pull the latest runs (#16033) * Update Multinode Warning (#16091) * [App] Serve datatypes with better client code (#16018) * docs: add PT version (#16010) * docs: add PT version * stable Co-authored-by: Adrian Wälchli <[email protected]> Co-authored-by: Adrian Wälchli <[email protected]> * add 1.13.1 to adjust versions (#16099) * Remove redundant `find_unused_parameters=False` in Lite (#16026) * [App] Add display name property to the work (#16095) Co-authored-by: thomas <[email protected]> * Fix detection of whether app is running in cloud (#16045) * [App] Add work.delete (#16103) Co-authored-by: thomas <[email protected]> * [App] Improve the autoscaler UI (#16063) [App] Improve the autoscaler UI (#16063) * Re-enable Lite CLI on Windows + PyTorch 1.13 (#15645) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Justus Schock <[email protected]> * [App] Min replica=0 would break autoscaler component (#16092) * fixing the bug where num_replica=0 would fail * changelog * [App] Scale out/in interval for autoscaler (#16093) * Adding arguments for scale out/in interval * Tests * Set the default work start method to spawn on MacOS (#16089) * [App] Add status endpoint, enable `ready` (#16075) Co-authored-by: thomas chaton <[email protected]> * Clarify `work.stop()` limitation (#16073) * fix merge errors * Update torchvision requirement from <=0.14.0,>=0.11.1 to >=0.11.1,<0.15.0 in /requirements (#16108) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Jirka <[email protected]> * CI: settle file names (#16098) * CI: settle file names * rename * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * Fix test failing on master due to bad auto-merge (#16118) * fix merge error Co-authored-by: Carlos Mocholí <[email protected]> Co-authored-by: Akihiro Nitta <[email protected]> Co-authored-by: thomas chaton <[email protected]> Co-authored-by: thomas <[email protected]> Co-authored-by: Yurij Mikhalevich <[email protected]> Co-authored-by: Ethan Harris <[email protected]> Co-authored-by: Sean Naren <[email protected]> Co-authored-by: Qiushi Pan <[email protected]> Co-authored-by: Jirka Borovec <[email protected]> Co-authored-by: Sherin Thomas <[email protected]> Co-authored-by: Justus Schock <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Jirka <[email protected]>

carmocca added 2 commits December 15, 2022 20:14

Better check for programmatic lightningignore

f0f65a0

Fix

11c4fb7

carmocca added bug Something isn't working app (removed) Generic label for Lightning App package components labels Dec 15, 2022

carmocca added this to the v1.8.x milestone Dec 15, 2022

carmocca requested a review from tchaton as a code owner December 15, 2022 19:17

carmocca self-assigned this Dec 15, 2022

carmocca requested review from lantiga, awaelchli, hhsecond and ethanwharris as code owners December 15, 2022 19:17

carmocca and others added 5 commits December 15, 2022 20:19

CHANGELOG

f3089d4

Restore multinode testing

7237941

[pre-commit.ci] auto fixes from pre-commit.com hooks

1dda5ae

for more information, see https://pre-commit.ci

Update num nodes

b44045b

Further fixes

3d4ac6d

carmocca force-pushed the bug/better-check-lightningignore branch from 06fe820 to 3d4ac6d Compare December 15, 2022 20:21

carmocca added 2 commits December 15, 2022 21:31

Skipif mark

0ea1f22

Formatting

0e32678

carmocca requested review from Borda and akihironitta as code owners December 15, 2022 20:44

awaelchli approved these changes Dec 15, 2022

View reviewed changes

ethanwharris approved these changes Dec 15, 2022

View reviewed changes

mergify bot added the ready PRs ready to be merged label Dec 15, 2022

carmocca force-pushed the bug/better-check-lightningignore branch 3 times, most recently from 1e1417b to 2e080e7 Compare December 15, 2022 23:59

Docs

59aab9f

carmocca force-pushed the bug/better-check-lightningignore branch from 2e080e7 to da95d70 Compare December 16, 2022 00:11

carmocca requested a review from williamFalcon as a code owner December 16, 2022 00:11

carmocca force-pushed the bug/better-check-lightningignore branch from da95d70 to 4b4323a Compare December 16, 2022 00:13

Avoid --editable mode since it adds all src modules to the path

c0dc817

carmocca force-pushed the bug/better-check-lightningignore branch from 4b4323a to c0dc817 Compare December 16, 2022 00:16

Borda reviewed Dec 16, 2022

View reviewed changes

.github/workflows/ci-app-examples.yml Outdated Show resolved Hide resolved

Apply suggestions from code review

a08e9b4

Borda approved these changes Dec 16, 2022

View reviewed changes

mergify bot added the has conflicts label Dec 16, 2022

Merge branch 'master' into bug/better-check-lightningignore

1120f68

mergify bot added ready PRs ready to be merged and removed has conflicts ready PRs ready to be merged labels Dec 16, 2022

carmocca enabled auto-merge (squash) December 16, 2022 03:09

Borda mentioned this pull request Dec 16, 2022

release: 1.8.5.post #16086

Merged

12 tasks

tchaton approved these changes Dec 16, 2022

View reviewed changes

carmocca merged commit b1ce263 into master Dec 16, 2022

carmocca deleted the bug/better-check-lightningignore branch December 16, 2022 09:49

Borda pushed a commit that referenced this pull request Dec 16, 2022

Better check for programmatic lightningignore (#16080)

4ecb340

Co-authored-by: Jirka Borovec <[email protected]> (cherry picked from commit b1ce263)

awaelchli pushed a commit that referenced this pull request Dec 19, 2022

Better check for programmatic lightningignore (#16080)

3485747

Co-authored-by: Jirka Borovec <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Better check for programmatic lightningignore #16080

Better check for programmatic lightningignore #16080

carmocca commented Dec 15, 2022 •

edited by github-actions bot

Loading

github-actions bot commented Dec 15, 2022 •

edited

Loading

tchaton left a comment

Better check for programmatic lightningignore #16080

Better check for programmatic lightningignore #16080

Conversation

carmocca commented Dec 15, 2022 • edited by github-actions bot Loading

What does this PR do?

Does your PR introduce any breaking changes? If yes, please list them.

github-actions bot commented Dec 15, 2022 • edited Loading

⚡ Required checks status: All passing 🟢

Groups summary

tchaton left a comment

Choose a reason for hiding this comment

carmocca commented Dec 15, 2022 •

edited by github-actions bot

Loading

github-actions bot commented Dec 15, 2022 •

edited

Loading