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.

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

pytorch_lightning mixed with lightning.pytorch: Getting a "ValueError('Expected a parent')" when using a list of Callbacks #96

Closed
CedricLeon opened this issue Mar 21, 2024 · 4 comments · Fixed by #97 or #99

Comments

@CedricLeon
Copy link
Contributor

Context

I see that you are using the old version of Pytorch Lightning in the lightning hooks:

from pytorch_lightning import LightningModule, Trainer
from pytorch_lightning.callbacks import Callback

There was some change in lightning a while ago and they renamed the package from pytorch_lightning to lightning. This actually
made quite a mess (you can check here for more details).

Error

From the update, mixing pytorch_lightning with lightning.pytorch is causing an error.
For example when creating a list of Callbacks, some depending on pytorch_lightning.Callbacks and other on lightning.pytorch.Callbacks.
This is what happened to me when I tried to add TriggerWandbSyncLightningCallback to my Trainer.
Here is the generated error:

hydra.errors.InstantiationException: Error in call to target 'lightning.pytorch.trainer.trainer.Trainer':
ValueError('Expected a parent')
full_key: trainer

(I am using hydra and several libraries who made the change to pytorch_lightning)

You can find more detals in the issue #17485.
Also, you can see a similar update/fix in the PR #5028 of Optuna.

Fix

I think simply replacing:

from pytorch_lightning import LightningModule, Trainer
from pytorch_lightning.callbacks import Callback

by

from lightning.pytorch import LightningModule, Trainer
from lightning.pytorch.callbacks import Callback

would do the trick

I will try opening a PR to fix that.
Thanks for your work and have a nice day!

@klieret
Copy link
Owner

klieret commented Mar 21, 2024

Thank you @CedricLeon for this very verbose report! I really appreciate that you added all the references, too! ❤️

@klieret
Copy link
Owner

klieret commented Mar 21, 2024

Seems like we need to be a bit more careful. Our setup.cfg currently has pytorch-lightning as a dependency (if you do pip install 'wandb-osh[lightning]', so the tests currently fail. However, pip install lightning also pulls in pytorch-lightning in (as a backwards compatibility layer), so I think we can just add lightning instead and it won't break anyone's code.

As for the imports in the module, I think I'll still play it safe and do a try except clause to test both import strategies (and perhaps display a warning if we fall back to the import pytorch_lightning).

A mess, indeed

@klieret
Copy link
Owner

klieret commented Mar 21, 2024

The new version on pypi (1.2.2) should have this fixed now! Can you let me know if it works for you?

@klieret klieret closed this as completed Mar 21, 2024
@CedricLeon
Copy link
Contributor Author

Seems like we need to be a bit more careful. Our setup.cfg currently has pytorch-lightning as a dependency (if you do pip install 'wandb-osh[lightning]', so the tests currently fail. However, pip install lightning also pulls in pytorch-lightning in (as a backwards compatibility layer), so I think we can just add lightning instead and it won't break anyone's code.

As for the imports in the module, I think I'll still play it safe and do a try except clause to test both import strategies (and perhaps display a warning if we fall back to the import pytorch_lightning).

A mess, indeed

Oh, I was too fast about that, I skipped the package management 😅 sorry.
I just updated the package, this works nicely! Thanks for the the fast support❤

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants