Skip to content

Conversation

@pujariaditya
Copy link

This PR fixes wandb integration issues in MetricsLogger reported in #137:

  1. Prevents wandb re-initialization: Added check to ensure MetricsLogger doesn't initialize wandb if a run is already active, fixing "You must call wandb.init() before wandb.log()" error when users manually initialize wandb.

  2. Ensures monotonic step tracking: Implemented global step counter _last_steps that tracks steps per metric prefix, preventing "Tried to log to step 0 that is less than the current step 3" warnings.

  3. Conditional wandb.finish(): Only finishes wandb runs if they were started by MetricsLogger, avoiding interference with user-managed sessions.

Reference

Checklist

  • I have verified that my change does not break existing code and all unit tests pass.
  • I have added all appropriate doc-strings/documentation.
  • My PR is based on the latest changes of the main branch (if unsure, rebase the code).
  • I have signed the Contributor License Agreement.
  • I have followed Contribution Guidelines.

- Add check to prevent re-initializing wandb if already active
- Implement global step tracking to ensure monotonic step numbers
- Only finish wandb runs if they were started by MetricsLogger
- Fixes issues with user-managed wandb sessions and step counting warnings
@darsnack
Copy link

It would be great if these changes could be merged.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants