Skip to content

Commit

Permalink
Better check for programmatic lightningignore (#16080)
Browse files Browse the repository at this point in the history
Co-authored-by: Jirka Borovec <6035284+Borda@users.noreply.github.com>
  • Loading branch information
carmocca and Borda authored Dec 16, 2022
1 parent 5f7403e commit b1ce263
Show file tree
Hide file tree
Showing 17 changed files with 46 additions and 73 deletions.
3 changes: 2 additions & 1 deletion .github/workflows/ci-app-examples.yml
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,8 @@ jobs:
- name: Install Lightning package
env:
PACKAGE_NAME: ${{ matrix.pkg-name }}
run: pip install -e .
# do not use -e because it will make both packages available since it adds `src` to `sys.path` automatically
run: pip install .

- name: Adjust tests
if: ${{ matrix.pkg-name == 'lightning' }}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ def run(self):
trainer = L.Trainer(max_epochs=10, strategy="ddp")
trainer.fit(model)

# 8 GPU: (2 nodes of 4 x v100)
# 8 GPUs: (2 nodes of 4 x v100)
component = LightningTrainerMultiNode(
LightningTrainerDistributed,
num_nodes=4,
Expand Down
2 changes: 1 addition & 1 deletion examples/app_multi_node/train_lite.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def run(self):
optimizer.step()


# Run over 2 nodes of 4 x V100
# 8 GPUs: (2 nodes of 4 x v100)
app = L.LightningApp(
LiteMultiNode(
LitePyTorchDistributed,
Expand Down
4 changes: 2 additions & 2 deletions examples/app_multi_node/train_lt.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ def run(self):
trainer.fit(model)


# 8 GPU: (2 nodes of 4 x v100)
# 8 GPUs: (2 nodes of 4 x v100)
component = LightningTrainerMultiNode(
LightningTrainerDistributed,
num_nodes=4,
num_nodes=2,
cloud_compute=L.CloudCompute("gpu-fast-multi"), # 4 x v100
)
app = L.LightningApp(component)
4 changes: 2 additions & 2 deletions examples/app_multi_node/train_lt_script.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@
from lightning.app.components import LightningTrainerScript
from lightning.app.utilities.packaging.cloud_compute import CloudCompute

# Run over 2 nodes of 4 x V100
# 8 GPUs: (2 nodes of 4 x v100)
app = L.LightningApp(
LightningTrainerScript(
"pl_boring_script.py",
num_nodes=2,
cloud_compute=CloudCompute("gpu-fast-multi"),
cloud_compute=CloudCompute("gpu-fast-multi"), # 4 x v100
),
)
2 changes: 1 addition & 1 deletion examples/app_multi_node/train_pytorch.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,6 @@ def run(self, main_address: str, main_port: int, num_nodes: int, node_rank: int)


# 8 GPUs: (2 nodes x 4 v 100)
compute = L.CloudCompute("gpu-fast-multi") # 4xV100
compute = L.CloudCompute("gpu-fast-multi") # 4 x v100
component = MultiNode(PyTorchDistributed, num_nodes=2, cloud_compute=compute)
app = L.LightningApp(component)
4 changes: 2 additions & 2 deletions examples/app_multi_node/train_pytorch_spawn.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,11 @@ def run(
optimizer.step()


# Run over 2 nodes of 4 x V100
# 8 GPUs: (2 nodes x 4 v 100)
app = L.LightningApp(
PyTorchSpawnMultiNode(
PyTorchDistributed,
num_nodes=2,
cloud_compute=L.CloudCompute("gpu-fast-multi"), # 4 x V100
cloud_compute=L.CloudCompute("gpu-fast-multi"), # 4 x v100
)
)
4 changes: 4 additions & 0 deletions src/lightning_app/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).

- Fixed the debugger detection mechanism for lightning App in VSCode ([#16068](https://github.com/Lightning-AI/lightning/pull/16068))


- Fixed bug where components that are re-instantiated several times failed to initialize if they were modifying `self.lightningignore` ([#16080](https://github.com/Lightning-AI/lightning/pull/16080))


- Fixed a bug where apps that had previously been deleted could not be run again from the CLI ([#16082](https://github.com/Lightning-AI/lightning/pull/16082))


Expand Down
4 changes: 2 additions & 2 deletions src/lightning_app/cli/lightning_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -358,8 +358,8 @@ def run_app(
)


if RequirementCache("lightning-lite"):
# lightning-lite may not be available when installing only standalone lightning-app package
if RequirementCache("lightning-lite>=1.9.0.dev0") or RequirementCache("lightning>=1.9.0.dev0"):
# lightning.lite.cli may not be available when installing only standalone lightning-app package
from lightning_lite.cli import _run_model

run.add_command(_run_model)
Expand Down
10 changes: 2 additions & 8 deletions src/lightning_app/core/flow.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,7 @@
from lightning_app.frontend import Frontend
from lightning_app.storage import Path
from lightning_app.storage.drive import _maybe_create_drive, Drive
from lightning_app.utilities.app_helpers import (
_is_json_serializable,
_lightning_dispatched,
_LightningAppRef,
_set_child_name,
is_overridden,
)
from lightning_app.utilities.app_helpers import _is_json_serializable, _LightningAppRef, _set_child_name, is_overridden
from lightning_app.utilities.component import _sanitize_state
from lightning_app.utilities.exceptions import ExitAppException
from lightning_app.utilities.introspection import _is_init_context, _is_run_context
Expand Down Expand Up @@ -325,7 +319,7 @@ def lightningignore(self) -> Tuple[str, ...]:

@lightningignore.setter
def lightningignore(self, lightningignore: Tuple[str, ...]) -> None:
if _lightning_dispatched():
if self._backend is not None:
raise RuntimeError(
f"Your app has been already dispatched, so modifying the `{self.name}.lightningignore` does not have an"
" effect"
Expand Down
9 changes: 2 additions & 7 deletions src/lightning_app/core/work.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,7 @@
from lightning_app.storage import Path
from lightning_app.storage.drive import _maybe_create_drive, Drive
from lightning_app.storage.payload import Payload
from lightning_app.utilities.app_helpers import (
_is_json_serializable,
_lightning_dispatched,
_LightningAppRef,
is_overridden,
)
from lightning_app.utilities.app_helpers import _is_json_serializable, _LightningAppRef, is_overridden
from lightning_app.utilities.component import _is_flow_context, _sanitize_state
from lightning_app.utilities.enum import (
CacheCallsKeys,
Expand Down Expand Up @@ -267,7 +262,7 @@ def lightningignore(self) -> Tuple[str, ...]:

@lightningignore.setter
def lightningignore(self, lightningignore: Tuple[str, ...]) -> None:
if _lightning_dispatched():
if self._backend is not None:
raise RuntimeError(
f"Your app has been already dispatched, so modifying the `{self.name}.lightningignore` does not have an"
" effect"
Expand Down
2 changes: 1 addition & 1 deletion tests/tests_app/components/multi_node/test_trainer.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ def test_trainer_run_executor_mps_forced_cpu(accelerator_given, accelerator_expe
({"strategy": "ddp_sharded_spawn"}, {"strategy": "ddp_sharded"}),
],
)
@pytest.mark.skipif(not module_available("pytorch"), reason="Lightning is not available")
@pytest.mark.skipif(not module_available("torch"), reason="PyTorch is not available")
def test_trainer_run_executor_arguments_choices(
args_given: dict,
args_expected: dict,
Expand Down
4 changes: 0 additions & 4 deletions tests/tests_app/runners/test_cloud.py
Original file line number Diff line number Diff line change
Expand Up @@ -1582,8 +1582,6 @@ def run(self):


def test_programmatic_lightningignore(monkeypatch, caplog, tmpdir):
monkeypatch.setenv("LIGHTNING_DISPATCHED", "0") # this is not cleaned up

mock_client = mock.MagicMock()
mock_client.projects_service_list_memberships.return_value = V1ListMembershipsResponse(
memberships=[V1Membership(name="test-project", project_id="test-project-id")]
Expand Down Expand Up @@ -1650,8 +1648,6 @@ def run(self):
assert "2 files were uploaded" # a.txt and .lightningignore
assert "files:\n5.0 MB: a.txt\nPerhaps" in caplog.text # only this file appears

# replicate how the app would dispatch the app, and call `run`
monkeypatch.setenv("LIGHTNING_DISPATCHED", "1")
flow.run()


Expand Down
10 changes: 5 additions & 5 deletions tests/tests_examples_app/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@

import psutil
import pytest
from tests_examples_app.public import _PATH_EXAMPLES

from lightning_app import _PROJECT_ROOT
from lightning_app.storage.path import _storage_root_dir
from lightning_app.utilities.component import _set_context
from lightning_app.utilities.packaging import cloud_compute
Expand All @@ -24,11 +24,11 @@ def pytest_sessionstart(*_):
"""Pytest hook that get called after the Session object has been created and before performing collection and
entering the run test loop."""
for name, url in GITHUB_APP_URLS.items():
if not os.path.exists(os.path.join(_PROJECT_ROOT, "examples", name)):
path_examples = os.path.join(_PROJECT_ROOT, "examples")
Popen(["git", "clone", url, name], cwd=path_examples).wait(timeout=90)
app_path = _PATH_EXAMPLES / name
if not os.path.exists(app_path):
Popen(["git", "clone", url, name], cwd=_PATH_EXAMPLES).wait(timeout=90)
else:
Popen(["git", "pull", "main"], cwd=os.path.join(_PROJECT_ROOT, "examples", name)).wait(timeout=90)
Popen(["git", "pull", "main"], cwd=app_path).wait(timeout=90)


def pytest_sessionfinish(session, exitstatus):
Expand Down
6 changes: 2 additions & 4 deletions tests/tests_examples_app/local/__init__.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import os
from pathlib import Path

from lightning_app import _PROJECT_ROOT

_PATH_APPS = os.path.join(_PROJECT_ROOT, "tests", "tests_examples_app", "apps")
_PATH_APPS = Path(__file__).resolve().parents[1] / "apps"
6 changes: 2 additions & 4 deletions tests/tests_examples_app/public/__init__.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import os
from pathlib import Path

from lightning_app import _PROJECT_ROOT

_PATH_EXAMPLES = os.path.join(_PROJECT_ROOT, "examples")
_PATH_EXAMPLES = Path(__file__).resolve().parents[3] / "examples"
43 changes: 15 additions & 28 deletions tests/tests_examples_app/public/test_multi_node.py
Original file line number Diff line number Diff line change
@@ -1,60 +1,47 @@
import os
import sys
from unittest import mock

import pytest
from lightning_utilities.core.imports import package_available
from tests_examples_app.public import _PATH_EXAMPLES

from lightning_app.testing.helpers import _RunIf
from lightning_app.testing.testing import application_testing, LightningTestApp


class LightningTestMultiNodeApp(LightningTestApp):
def on_before_run_once(self):
res = super().on_before_run_once()
if self.works and all(w.has_stopped for w in self.works):
assert len([w for w in self.works]) == 2
assert len(self.works) == 2
return True
return res


@pytest.mark.skip(reason="flaky")
@mock.patch("lightning_app.components.multi_node.base.is_running_in_cloud", return_value=True)
def test_multi_node_example(_, monkeypatch):
monkeypatch.chdir(os.path.join(_PATH_EXAMPLES, "app_multi_node"))
command_line = [
"app.py",
"--blocking",
"False",
"--open-ui",
"False",
]
result = application_testing(LightningTestMultiNodeApp, command_line)
assert result.exit_code == 0


class LightningTestMultiNodeWorksApp(LightningTestApp):
def on_before_run_once(self):
res = super().on_before_run_once()
if self.works and all(w.has_stopped for w in self.works):
assert len([w for w in self.works]) == 2
return True
return res
# for the skip to work, the package needs to be installed without editable mode
_SKIP_LIGHTNING_UNAVAILABLE = pytest.mark.skipif(not package_available("lightning"), reason="script requires lightning")


@pytest.mark.parametrize(
"app_name",
[
"train_pytorch.py",
"train_any.py",
# "app_lite_work.py",
"train_pytorch_spawn.py",
# "app_pl_work.py": TODO Add once https://github.com/Lightning-AI/lightning/issues/15556 is resolved.
pytest.param("train_lite.py", marks=_SKIP_LIGHTNING_UNAVAILABLE),
pytest.param("train_lt_script.py", marks=_SKIP_LIGHTNING_UNAVAILABLE),
pytest.param("train_lt.py", marks=_SKIP_LIGHTNING_UNAVAILABLE),
],
)
@pytest.mark.skipif(sys.platform == "win32", reason="flaky")
@_RunIf(skip_windows=True) # flaky
@mock.patch("lightning_app.components.multi_node.base.is_running_in_cloud", return_value=True)
def test_multi_node_examples(_, app_name, monkeypatch):
# note: this test will fail locally:
# * if you installed `lightning_app`, then the examples need to be
# rewritten to use `lightning_app` imports (CI does this)
# * if you installed `lightning`, then the imports in this file and mocks
# need to be changed to use `lightning`.
monkeypatch.chdir(os.path.join(_PATH_EXAMPLES, "app_multi_node"))
command_line = [app_name, "--blocking", "False", "--open-ui", "False", "--setup"]
result = application_testing(LightningTestMultiNodeWorksApp, command_line)
result = application_testing(LightningTestMultiNodeApp, command_line)
assert result.exit_code == 0

0 comments on commit b1ce263

Please sign in to comment.