pytorch-lightning

pytorch-lightning copied to clipboard

🐛 Bug

Following the example for using a public docker image results in an error. I modified the work slightly just to print hi.

To Reproduce

Component.py

from lightning_app import LightningWork, BuildConfig

class MyWork(LightningWork):
    def __init__(self):
        super().__init__()

        # Using a publicly hosted docker image:
        self.cloud_build_config = BuildConfig(
            # This is one of the base images Lightning uses by default
            image="ghcr.io/gridai/base-images:v1.8-gpu"
        )

    def run(self):
        print('hi')

app.py

import lightning as L
from lightning_app.storage import Drive
from components.fail import MyWork


class LitApp(L.LightningFlow):
    def __init__(self) -> None:
        super().__init__()
        self.work = MyWork()

    def run(self):
        self.work.run()


app = L.LightningApp(LitApp())

error

Your Lightning App is starting. This won't take long.
ERROR: Found an exception when loading your application from fail.py. Please, resolve it to run your app.

Traceback (most recent call last):
  File "fail.py", line 15, in <module>
    app = L.LightningApp(LitApp())
  File "fail.py", line 9, in __init__
    self.work = MyWork()
  File "C:\Users\Olufemi Ojo\Documents\MlFlowComponent\components\fail.py", line 8, in __init__
    self.cloud_build_config = BuildConfig(
  File "C:\Users\Olufemi Ojo\Documents\envs\network\lib\site-packages\lightning_app\core\work.py", line 300, in __setattr__
    setattr_fn(name, value)
  File "C:\Users\Olufemi Ojo\Documents\envs\network\lib\site-packages\lightning_app\core\work.py", line 365, in _default_setattr
    raise AttributeError(
AttributeError: Only JSON-serializable attributes are currently supported (str, int, float, bool, tuple, list, dict etc.) to be part of <components.fail.MyWork object at 0x000002D6446D0070> state. Found the attribute cloud_build_config with BuildConfig(requirements=None, dockerfile=None, image='ghcr.io/gridai/base-images:v1.8-gpu') instead.
HINT: Private attributes defined as follows `self._x = y` won't be shared between components and therefore don't need to be JSON-serializable. If you need to include non-JSON serializable objects in the state, you can use the `lightning_app.storage.Payload` API.

Expected behavior

no error following the docs

Environment

• CUDA: - GPU: - available: False - version: None
• Packages: - lightning: 2022.7.18 - lightning_app: 0.5.2 - numpy: 1.23.1 - pyTorch_debug: False - pyTorch_version: 1.12.0+cpu - pytorch-lightning: 1.6.5 - tqdm: 4.64.0
• System: - OS: Windows - architecture: - 64bit - WindowsPE - processor: Intel64 Family 6 Model 140 Stepping 1, GenuineIntel - python: 3.9.7 - version: 10.0.22000

cc @tchaton @rohitgr7

A simple fix for this issue seems to be adding a __json__ attribute to BuildConfig class.

The __json__ attribute would look like this,

    def __json__(self):
        return self.to_dict()

If this solution seems right, I could quickly make PR to fix this issue.

Any comments/suggestions would be really helpful! @manskx @akihironitta @tchaton

Out of curiosity, how did you arrive at that solution?

On Sat, Aug 6, 2022, 8:32 AM Pranjal Datta @.***> wrote:

A simple fix for this issue seems to be adding a json attribute to BuildConfig class https://github.com/Lightning-AI/lightning/blob/26d69ceada7f4ad1632e70df6414348170e85574/src/lightning_app/utilities/packaging/build_config.py#L49 .

The json attribute would look like this,

def __json__(self):
    return self.to_dict()

If this solution seems right, I could quickly make PR to fix this issue.

Any comments/suggestions would be really helpful! @manskx https://github.com/manskx @akihironitta https://github.com/akihironitta @tchaton https://github.com/tchaton

— Reply to this email directly, view it on GitHub https://github.com/Lightning-AI/lightning/issues/13934#issuecomment-1207206699, or unsubscribe https://github.com/notifications/unsubscribe-auth/ALHYMCXXMIBXZ56IMBH6BVDVXZLNTANCNFSM55BOGX4A . You are receiving this because you authored the thread.Message ID: @.***>

Out of curiosity, how did you arrive at that solution? … On Sat, Aug 6, 2022, 8:32 AM Pranjal Datta @.> wrote: A simple fix for this issue seems to be adding a json attribute to BuildConfig class https://github.com/Lightning-AI/lightning/blob/26d69ceada7f4ad1632e70df6414348170e85574/src/lightning_app/utilities/packaging/build_config.py#L49 . The json attribute would look like this, def json(self): return self.to_dict() If this solution seems right, I could quickly make PR to fix this issue. Any comments/suggestions would be really helpful! @manskx https://github.com/manskx @akihironitta https://github.com/akihironitta @tchaton https://github.com/tchaton — Reply to this email directly, view it on GitHub <#13934 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ALHYMCXXMIBXZ56IMBH6BVDVXZLNTANCNFSM55BOGX4A . You are receiving this because you authored the thread.Message ID: @.>

So the AttributeError is being thrown here since, _is_json_serializable returns False as this function in turns tries to do a json.dumps here using a custom JSON Encoder class defined here. It is this encoder class looks for a __json__ attribute.

So the issue here is caused by the asymmetry between python's __getattr__ and __setattr__ behaviour. The former is called only if the attribute is not found but the latter is called regardless.

The work class already has a property.setter for build configs defined which is supposed to be called when you set the build config and this error would not have raised.

    @cloud_build_config.setter
    def cloud_build_config(self, build_config: BuildConfig) -> None:
        self._cloud_build_config = build_config
        self._cloud_build_config.on_work_init(self, cloud_compute=self._cloud_compute)

But instead what happens is that the custom __setattr__ defined is taking priority over the property.setter which checks for the object to be json serializable or not.

Probably a solution here would be to fix the __setattr__ function to check if the incoming name has a property setter defined or not. @pranjaldatta Would it be something you are interested in contributing to?

@oojo12 I have slightly modified the title. As a workaround to your problem, can you try passing the build config as an argument to the init call of super class.

from lightning_app import LightningWork, BuildConfig

class MyWork(LightningWork):
    def __init__(self):
        super().__init__(cloud_build_config=BuildConfig(image="ghcr.io/gridai/base-images:v1.8-gpu"))

    def run(self):
        print('hi')

So the issue here is caused by the asymmetry between python's __getattr__ and __setattr__ behaviour. The former is called only if the attribute is not found but the latter is called regardless.

The work class already has a property.setter for build configs defined which is supposed to be called when you set the build config and this error would not have raised.

    @cloud_build_config.setter
    def cloud_build_config(self, build_config: BuildConfig) -> None:
        self._cloud_build_config = build_config
        self._cloud_build_config.on_work_init(self, cloud_compute=self._cloud_compute)

But instead what happens is that the custom __setattr__ defined is taking priority over the property.setter which checks for the object to be json serializable or not.

Probably a solution here would be to fix the __setattr__ function to check if the incoming name has a property setter defined or not. @pranjaldatta Would it be something you are interested in contributing to?

@hhsecond, I would love to contribute to this issue! I tried to make some quick changes and it seems like this solution works. If this is cool with you, I could make a PR in a day or two

@pranjaldatta fantastic. Let's get this one fixed. Thanks a lot for taking it up.

@pranjaldatta circling back to see if you have got some time to spare on this one? Please don't hesitate to ask questions or seek help incase you need

@pranjaldatta circling back to see if you have got some time to spare on this one? Please don't hesitate to ask questions or seek help incase you need

Hi @hhsecond , so sorry about the delay! I have the code ready. It is just that I haven't had the time to make the PR due to some prior commitments. I will definitely try to to finish the PR by today.