Skip to content

Conversation

@carmocca
Copy link
Contributor

@carmocca carmocca commented Nov 21, 2022

What does this PR do?

Avoids importing tensorboard unless the logger is used.
Part of #12786
Part of #15662

Does your PR introduce any breaking changes? If yes, please list them.

None

cc @awaelchli @Borda @akihironitta

@carmocca carmocca added this to the v1.9 milestone Nov 21, 2022
@carmocca carmocca requested a review from awaelchli as a code owner November 21, 2022 23:51
@carmocca carmocca self-assigned this Nov 21, 2022
@github-actions github-actions bot added the pl Generic label for PyTorch Lightning package label Nov 21, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Nov 21, 2022

⚡ Required checks status: All passing 🟢

Groups summary

🟢 pytorch_lightning: Tests workflow
Check ID Status
pl-cpu (macOS-11, pytorch, 3.8, 1.11) success
pl-cpu (macOS-11, pytorch, 3.9, 1.12) success
pl-cpu (macOS-11, pytorch, 3.10, 1.13) success
pl-cpu (macOS-11, pytorch, 3.8, 1.10, oldest) success
pl-cpu (ubuntu-20.04, pytorch, 3.8, 1.10) success
pl-cpu (ubuntu-20.04, pytorch, 3.9, 1.11) success
pl-cpu (ubuntu-20.04, pytorch, 3.10, 1.12) success
pl-cpu (ubuntu-20.04, pytorch, 3.10, 1.13) success
pl-cpu (ubuntu-20.04, pytorch, 3.7, 1.10, oldest) success
pl-cpu (windows-2022, pytorch, 3.9, 1.11) success
pl-cpu (windows-2022, pytorch, 3.10, 1.12) success
pl-cpu (windows-2022, pytorch, 3.10, 1.13) success
pl-cpu (windows-2022, pytorch, 3.7, 1.10, oldest) success
pl-cpu (slow, macOS-11, pytorch, 3.7, 1.11) success
pl-cpu (slow, ubuntu-20.04, pytorch, 3.7, 1.11) success
pl-cpu (slow, windows-2022, pytorch, 3.7, 1.11) success
pl-cpu (macOS-11, lightning, 3.8, 1.13) success
pl-cpu (ubuntu-20.04, lightning, 3.8, 1.13) success
pl-cpu (windows-2022, lightning, 3.8, 1.13) success
🟢 pytorch_lightning: Azure GPU
Check ID Status
pytorch-lightning (GPUs) success
🟢 pytorch_lightning: Azure HPU
Check ID Status
pytorch-lightning (HPUs) success
🟢 pytorch_lightning: Azure IPU
Check ID Status
pytorch-lightning (IPUs) success
🟢 pytorch_lightning: Docs
Check ID Status
make-doctest (pytorch) success
make-html (pytorch) success
🟢 mypy
Check ID Status
mypy success
🟢 install
Check ID Status
install-pkg (ubuntu-22.04, app, 3.7) success
install-pkg (ubuntu-22.04, app, 3.10) success
install-pkg (ubuntu-22.04, lite, 3.7) success
install-pkg (ubuntu-22.04, lite, 3.10) success
install-pkg (ubuntu-22.04, pytorch, 3.7) success
install-pkg (ubuntu-22.04, pytorch, 3.10) success
install-pkg (ubuntu-22.04, lightning, 3.7) success
install-pkg (ubuntu-22.04, lightning, 3.10) success
install-pkg (macOS-12, app, 3.7) success
install-pkg (macOS-12, app, 3.10) success
install-pkg (macOS-12, lite, 3.7) success
install-pkg (macOS-12, lite, 3.10) success
install-pkg (macOS-12, pytorch, 3.7) success
install-pkg (macOS-12, pytorch, 3.10) success
install-pkg (macOS-12, lightning, 3.7) success
install-pkg (macOS-12, lightning, 3.10) success
install-pkg (windows-2022, app, 3.7) success
install-pkg (windows-2022, app, 3.10) success
install-pkg (windows-2022, lite, 3.7) success
install-pkg (windows-2022, lite, 3.10) success
install-pkg (windows-2022, pytorch, 3.7) success
install-pkg (windows-2022, pytorch, 3.10) success
install-pkg (windows-2022, lightning, 3.7) success
install-pkg (windows-2022, lightning, 3.10) success

This comment was automatically generated and updates for 60 minutes every 180 seconds.

Thank you for your contribution! 💜

if _TENSORBOARD_AVAILABLE:
from torch.utils.tensorboard.summary import hparams
else:
from tensorboardX.summary import hparams # type: ignore[no-redef]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how many times is this called per epoch?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was answered in #15762 (comment)

@mergify mergify bot added the ready PRs ready to be merged label Nov 23, 2022
@carmocca carmocca merged commit a970810 into master Nov 23, 2022
@carmocca carmocca deleted the refactor/lazy-import-tb branch November 23, 2022 12:59
_TENSORBOARDX_AVAILABLE = RequirementCache("tensorboardX")
if TYPE_CHECKING:
# assumes at least one will be installed when type checking
if _TENSORBOARD_AVAILABLE:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is opposite what @lantiga said in #15728 (comment) 🦦

sub_dir: Optional[_PATH] = None,
**kwargs: Any,
):
if not _TENSORBOARD_AVAILABLE and not _TENSORBOARDX_AVAILABLE:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this won't happen as TBX is mandatory depenency

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

logger: tensorboard performance pl Generic label for PyTorch Lightning package ready PRs ready to be merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants