Skip to content

Conversation

@DanielNobbe
Copy link
Contributor

@DanielNobbe DanielNobbe commented Dec 4, 2025

Fixes #3471

Description:
Remove the sync argument from the wandb.log() calls in the OutputHandler and OptimizerParamsHandler, since the argument was removed. Kept the argument in the initialisation of the handlers, so that existing code that may include the argument keeps working.

Check list:

  • New tests are added (if a new feature is added)
  • New doc strings: description and/or example code are in RST format
  • Documentation is updated (if required)

@github-actions github-actions bot added the module: handlers Core Handlers module label Dec 4, 2025
Updated sync parameter documentation to indicate deprecation.
@DanielNobbe DanielNobbe marked this pull request as ready for review December 4, 2025 17:17
Copy link
Collaborator

@vfdev-5 vfdev-5 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @DanielNobbe !

@vfdev-5
Copy link
Collaborator

vfdev-5 commented Dec 5, 2025

@DanielNobbe can you also please remove sync arg usage from tests/ignite/handlers/test_wandb_logger.py. Thanks!
and also fix the code style checker error: https://github.com/pytorch/ignite/actions/runs/19937571924/job/57228045157?pr=3472

@DanielNobbe
Copy link
Contributor Author

Done. I didn't think of checking the tests, sorry about that.
I removed the one test that checked for the functionality of the sync argument, since the argument does nothing anymore. We don't have a test verifying that the sync argument does not trigger any errors, but I don't think that's necessary. If you would like me to add that test, let me know.

@vfdev-5
Copy link
Collaborator

vfdev-5 commented Dec 5, 2025

Thanks @DanielNobbe ! That's fine as you did. Let's see if the CI is green and we can land it.

@vfdev-5 vfdev-5 added this pull request to the merge queue Dec 5, 2025
@vfdev-5 vfdev-5 removed this pull request from the merge queue due to a manual request Dec 5, 2025
@vfdev-5 vfdev-5 merged commit f400030 into pytorch:master Dec 5, 2025
21 of 24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

module: handlers Core Handlers module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

WandBLogger calls Run.log with sync argument

2 participants