Skip to content

Commit

Permalink
[App] Fix bug when using structures with works (#15911)
Browse files Browse the repository at this point in the history
* Fix bug when using structures with works
* Add test
* Update CHANGELOG.md

(cherry picked from commit 1283226)
  • Loading branch information
ethanwharris authored and Borda committed Dec 8, 2022
1 parent 42494b6 commit 5a25156
Show file tree
Hide file tree
Showing 9 changed files with 37 additions and 13 deletions.
2 changes: 1 addition & 1 deletion requirements/app/base.txt
Expand Up @@ -7,7 +7,7 @@ fsspec>=2022.5.0, <=2022.7.1
croniter>=1.3.0, <1.4.0 # strict; TODO: for now until we find something more robust.
traitlets>=5.3.0, <=5.4.0
arrow>=1.2.0, <1.2.4
lightning-utilities>=0.3.*, !=0.4.0, <0.5.0
lightning-utilities>=0.3.0, !=0.4.0, <0.5.0
beautifulsoup4>=4.8.0, <4.11.2
inquirer>=2.10.0
psutil<5.9.4
Expand Down
2 changes: 1 addition & 1 deletion requirements/lite/base.txt
Expand Up @@ -2,7 +2,7 @@
# in case you want to preserve/enforce restrictions on the latest compatible version, add "strict" as an in-line comment

numpy>=1.17.2, <1.23.1
torch>=1.9.*, <=1.13.0
torch>=1.9.0, <=1.13.0
fsspec[http]>2021.06.0, <2022.6.0
packaging>=17.0, <=21.3
typing-extensions>=4.0.0, <=4.4.0
Expand Down
4 changes: 4 additions & 0 deletions requirements/lite/examples.txt
@@ -0,0 +1,4 @@
# NOTE: the upper bound for the package version is only set for CI stability, and it is dropped while installing this package
# in case you want to preserve/enforce restrictions on the latest compatible version, add "strict" as an in-line comment

torchvision>=0.10.0, <=0.13.0
2 changes: 1 addition & 1 deletion requirements/pytorch/base.txt
Expand Up @@ -2,7 +2,7 @@
# in case you want to preserve/enforce restrictions on the latest compatible version, add "strict" as an in-line comment

numpy>=1.17.2, <1.23.1
torch>=1.9.*, <=1.13.0
torch>=1.9.0, <=1.13.0
tqdm>=4.57.0, <4.65.0
PyYAML>=5.4, <=6.0
fsspec[http]>2021.06.0, <2022.8.0
Expand Down
2 changes: 1 addition & 1 deletion requirements/pytorch/examples.txt
@@ -1,6 +1,6 @@
# NOTE: the upper bound for the package version is only set for CI stability, and it is dropped while installing this package
# in case you want to preserve/enforce restrictions on the latest compatible version, add "strict" as an in-line comment

torchvision>=0.10.*, <=0.13.0
torchvision>=0.10.0, <=0.14.0
gym[classic_control]>=0.17.0, <0.26.3
ipython[all] <8.6.1
2 changes: 2 additions & 0 deletions src/lightning_app/CHANGELOG.md
Expand Up @@ -41,6 +41,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
- Fixed the `enable_spawn` method of the `WorkRunExecutor` ([#15812](https://github.com/Lightning-AI/lightning/pull/15812))
- Fixed require/import decorator ([#15849](https://github.com/Lightning-AI/lightning/pull/15849))

- Fixed a bug where using `L.app.structures` would cause multiple apps to be opened and fail with an error in the cloud ([#15911](https://github.com/Lightning-AI/lightning/pull/15911))


## [1.8.3] - 2022-11-22

Expand Down
2 changes: 1 addition & 1 deletion src/lightning_app/structures/dict.py
Expand Up @@ -64,10 +64,10 @@ def __setitem__(self, k, v):
if isinstance(k, str) and "." in k:
raise Exception(f"The provided name {k} contains . which is forbidden.")

_set_child_name(self, v, k)
if self._backend:
if isinstance(v, LightningFlow):
LightningFlow._attach_backend(v, self._backend)
_set_child_name(self, v, k)
elif isinstance(v, LightningWork):
self._backend._wrap_run_method(_LightningAppRef().get_current(), v)
v._name = f"{self.name}.{k}"
Expand Down
12 changes: 4 additions & 8 deletions src/lightning_app/structures/list.py
@@ -1,7 +1,5 @@
import typing as t

from pyparsing import Optional

from lightning_app.utilities.app_helpers import _LightningAppRef, _set_child_name

T = t.TypeVar("T")
Expand Down Expand Up @@ -52,23 +50,21 @@ def __init__(self, *items: T):

self._name: t.Optional[str] = ""
self._last_index = 0
self._backend: Optional[Backend] = None
self._backend: t.Optional[Backend] = None
for item in items:
self.append(item)
_set_child_name(self, item, str(self._last_index))
self._last_index += 1

def append(self, v):
from lightning_app import LightningFlow, LightningWork

_set_child_name(self, v, str(self._last_index))
if self._backend:
if isinstance(v, LightningFlow):
LightningFlow._attach_backend(v, self._backend)
_set_child_name(self, v, str(self._last_index))
elif isinstance(v, LightningWork):
self._backend._wrap_run_method(_LightningAppRef().get_current(), v)
v._name = f"{self.name}.{self._last_index}"
self._last_index += 1
v._name = f"{self.name}.{self._last_index}"
self._last_index += 1
super().append(v)

@property
Expand Down
22 changes: 22 additions & 0 deletions tests/tests_app/structures/test_structures.py
Expand Up @@ -496,3 +496,25 @@ def test_structures_with_payload():
app = LightningApp(FlowPayload(), log_level="debug")
MultiProcessRuntime(app, start_server=False).dispatch()
os.remove("payload")


def test_structures_have_name_on_init():
"""Test that the children in structures have the correct name assigned upon initialization."""

class ChildWork(LightningWork):
def run(self):
pass

class Collection(EmptyFlow):
def __init__(self):
super().__init__()
self.list_structure = List()
self.list_structure.append(ChildWork())

self.dict_structure = Dict()
self.dict_structure["dict_child"] = ChildWork()

flow = Collection()
LightningApp(flow) # wrap in app to init all component names
assert flow.list_structure[0].name == "root.list_structure.0"
assert flow.dict_structure["dict_child"].name == "root.dict_structure.dict_child"

0 comments on commit 5a25156

Please sign in to comment.