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

WandBLogger and TensorboardLogger have different APIs for logging audio #2791

Closed
turian opened this issue Dec 4, 2022 · 4 comments · Fixed by #2826
Closed

WandBLogger and TensorboardLogger have different APIs for logging audio #2791

turian opened this issue Dec 4, 2022 · 4 comments · Fixed by #2826

Comments

@turian
Copy link

turian commented Dec 4, 2022

🚀 Feature

The following code doesn't work:

logger = WandBLogger()
logger.writer

This is how you would typically add audio with a tensorboard logger:

logger.writer.add_audio('mixture', x.t(), engine.state.epoch)

The workaround (similar to discussed in Lightning-AI/pytorch-lightning#7028) would be to use the underlying _wandb object:

logger._wandb.log({"validation": [wandb.Audio(x.t(), caption="mixture", sample_rate=44100)]})
logger._wandb.log({"validation": [wandb.Audio(x.t(), caption="vocals", sample_rate=44100)]})

Is there a proposal for a standardized media logging API?

@turian turian changed the title WandBLogger doesn't support writer and cannot write audio WandBLogger and TensorboardLogger have different APIs for logging audio Dec 4, 2022
@vfdev-5
Copy link
Collaborator

vfdev-5 commented Dec 4, 2022

@turian thanks for reporting.

I think you do not need such workaround as we propagate automatically attributes to _wandb:

def __getattr__(self, attr: Any) -> Any:
return getattr(self._wandb, attr)

So, I think the following should work (haven't explicitly checked)

logger.log({"validation": [wandb.Audio(x.t(), caption="mixture", sample_rate=44100)]})
logger.log({"validation": [wandb.Audio(x.t(), caption="vocals", sample_rate=44100)]})

WandBLogger and TensorboardLogger have different APIs for logging audio

I agree that in case of TensorboardLogger, user can access logger.writer object and use directly tensorboard API as in your example. Maybe, it would make sense to propagate attributes to writer similarly to what we are doing for WandBLogger. What do you think ?

@guptaaryan16
Copy link
Contributor

Hey @vfdev-5 I want to contribute to this issue, can you please give me a general idea what should I do to make the APIs consistent

@vfdev-5
Copy link
Collaborator

vfdev-5 commented Jan 16, 2023

@guptaaryan16 as it is described in #2791 (comment) we can add __getattr__ to TensorboardLogger to propagate call to writer such that user does not need to call it:

- logger.writer.add_audio('mixture', x.t(), engine.state.epoch)
+ logger.add_audio('mixture', x.t(), engine.state.epoch)

Is it more clear now ?

@guptaaryan16
Copy link
Contributor

thanks for the guidance , I will try to complete it as fast as possible

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

Successfully merging a pull request may close this issue.

3 participants