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

Wrong warnings module used in lightning module #14231

Closed
roytseng-tw opened this issue Aug 16, 2022 · 1 comment · Fixed by #14234
Closed

Wrong warnings module used in lightning module #14231

roytseng-tw opened this issue Aug 16, 2022 · 1 comment · Fixed by #14234
Labels
bug Something isn't working lightningmodule pl.LightningModule logger Related to the Loggers
Milestone

Comments

@roytseng-tw
Copy link

roytseng-tw commented Aug 16, 2022

🐛 Bug

https://github.com/Lightning-AI/lightning/blob/2622989b108368813fdd3850b002fd1ad69988c2/src/pytorch_lightning/core/module.py#L41
https://github.com/Lightning-AI/lightning/blob/2622989b108368813fdd3850b002fd1ad69988c2/src/pytorch_lightning/core/module.py#L280-L282

L280-L282 intend to use python warnings module instead of the one defined in pytorch lightning.

To Reproduce

Use multiple loggers:
pl.trainer(loggers=[logger1, logger2], ...)

Expected behavior

Since pytorch_lightning.utilities.warnings isn't used anywhere in the file, the solution will be:

  1. remove the wrong warnings import.
  2. add import warnings

Environment

Doesn't really matter.

  • Lightning Component (e.g. Trainer, LightningModule, LightningApp, LightningWork, LightningFlow):
  • PyTorch Lightning Version (e.g., 1.5.0):
  • Lightning App Version (e.g., 0.5.2):
  • PyTorch Version (e.g., 1.10):
  • Python version (e.g., 3.9):
  • OS (e.g., Linux):
  • CUDA/cuDNN version:
  • GPU models and configuration:
  • How you installed PyTorch (conda, pip, source):
  • If compiling from source, the output of torch.__config__.show():
  • Running environment of LightningApp (e.g. local, cloud):
  • Any other relevant information:

Additional context

cc @awaelchli @edward-io @Borda @ananthsub @rohitgr7 @kamil-kaczmarek @Raalsky @Blaizzy @carmocca @justusschock @ninginthecloud @jjenniferdai

@roytseng-tw roytseng-tw added the needs triage Waiting to be triaged by maintainers label Aug 16, 2022
@roytseng-tw roytseng-tw changed the title Wrong warning module used in lightning module Wrong warnings module used in lightning module Aug 16, 2022
@awaelchli
Copy link
Contributor

@roytseng-tw I see what you mean. Your report is however incomplete. This is the repro:

    model = BoringModel()
    trainer = Trainer(
        logger=[TensorBoardLogger("."), TensorBoardLogger(".")]
    )
    model.trainer = trainer
    model.logger
Traceback (most recent call last):
  File "/Users/adrian/repositories/lightning/examples/pl_bug_report/bug_report_model.py", line 57, in <module>
    run()
  File "/Users/adrian/repositories/lightning/examples/pl_bug_report/bug_report_model.py", line 53, in run
    model.logger
  File "/Users/adrian/miniconda3/envs/lightning/lib/python3.10/site-packages/torch/nn/modules/module.py", line 1207, in __getattr__
    raise AttributeError("'{}' object has no attribute '{}'".format(
AttributeError: 'BoringModel' object has no attribute 'logger'. Did you mean: 'loggers'?

  1. We should fix this by importing warnings correctly as you said (thanks!!), and release this in 1.7.x
  2. We should get rid of the code completely because the deprecation phase is over in 1.8.

Thanks for raising this issue!

@awaelchli awaelchli added bug Something isn't working logger Related to the Loggers lightningmodule pl.LightningModule and removed needs triage Waiting to be triaged by maintainers labels Aug 16, 2022
@awaelchli awaelchli added this to the pl:1.7.x milestone Aug 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working lightningmodule pl.LightningModule logger Related to the Loggers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants