Skip to content

Removed sync arg from wandb.log#3472

Merged
vfdev-5 merged 4 commits intopytorch:masterfrom
DanielNobbe:wandb-logger-remove-sync
Dec 5, 2025
Merged

Removed sync arg from wandb.log#3472
vfdev-5 merged 4 commits intopytorch:masterfrom
DanielNobbe:wandb-logger-remove-sync

Conversation

@DanielNobbe
Copy link
Copy Markdown
Contributor

@DanielNobbe DanielNobbe commented Dec 4, 2025 •
edited by vfdev-5
Loading

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
@DanielNobbe
Deprecate sync parameter in wandb_logger
d0ff308 
Updated sync parameter documentation to indicate deprecation.
@DanielNobbe DanielNobbe marked this pull request as ready for review December 4, 2025 17:17
vfdev-5
vfdev-5 approved these changes Dec 5, 2025
View reviewed changes
Copy link
Copy Markdown
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
Copy Markdown
Collaborator

vfdev-5 commented Dec 5, 2025 •
edited
Loading

@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
Copy Markdown
Contributor Author

DanielNobbe commented Dec 5, 2025

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
Copy Markdown
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
Copilot AI mentioned this pull request Apr 1, 2026
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vfdev-5 vfdev-5 vfdev-5 approved these changes

Assignees

No one assigned

Labels

module: handlers Core Handlers module

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

WandBLogger calls Run.log with sync argument

2 participants

@DanielNobbe @vfdev-5