Skip to content

Conversation

@yzhangcs
Copy link
Contributor

This commit

  1. Move the tensorboard imports into class so that it will not raise errors if tensorboard not installed.
  2. Allow users to obtain wandb project name from the env variable WANDB_PROJECT, which is defined in their docs.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Meta Open Source bot. label Feb 25, 2025
Copy link
Contributor

@fegin fegin left a comment

Choose a reason for hiding this comment

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

I'm okay with the WANDB part. But from torch.utils.tensorboard is the standard PyTorch path not from another repro. So prefer not to use lazy initialization for this.

from typing import Any, Dict, Optional

import torch
from torch.utils.tensorboard import SummaryWriter
Copy link
Contributor

Choose a reason for hiding this comment

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

I have similar question. I'd imagine:

  1. torch.utils.tensorboard is available as long as torch is installed. And it can generate TB logs even if tensorboard library is not installed.
  2. tensorboard library can be used to render the TB logs through SSH tunneling.
    If that's the case, we can always keep torch.utils.tensorboard import.
    But I could be wrong.

@yzhangcs
Copy link
Contributor Author

@fegin @tianyu-l Thank you for the feedbacks, just revert the changes

@yzhangcs yzhangcs changed the title [Metrics] Don't rely on tensorboard dep & allow getting project nam… [Metrics] Allow getting wandb project name via WANDB_PROJECT Feb 26, 2025
Copy link
Contributor

@tianyu-l tianyu-l left a comment

Choose a reason for hiding this comment

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

lgtm

…es from env

This commit 
1. Move the `tensorboard` imports into class so that it will not raise errors if `tensorboard` not installed.
2. Allow users to obtain `wandb` project name from the env variable `WANDB_PROJECT`, which is defined in their docs.
@tianyu-l tianyu-l merged commit 6cb13c7 into pytorch:main Feb 26, 2025
3 checks passed
MaxiBoether pushed a commit to eth-easl/torchtitan-mixtera that referenced this pull request Apr 17, 2025
…ch#888)

This commit
1. Move the `tensorboard` imports into class so that it will not raise
errors if `tensorboard` not installed.
2. Allow users to obtain `wandb` project name from the env variable
`WANDB_PROJECT`, which is defined in their docs.
MaxiBoether pushed a commit to eth-easl/torchtitan-mixtera that referenced this pull request Apr 17, 2025
…ch#888)

This commit
1. Move the `tensorboard` imports into class so that it will not raise
errors if `tensorboard` not installed.
2. Allow users to obtain `wandb` project name from the env variable
`WANDB_PROJECT`, which is defined in their docs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants