-
-
Notifications
You must be signed in to change notification settings - Fork 908
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
Feat(wandb): Refactor to be more flexible #767
Feat(wandb): Refactor to be more flexible #767
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this PR make a major difference since it still points to the same keyword arg here?
Yes!
Edit: I noticed one edge case from reading their docs on run_id. During resume, it might not continue in the same training and show as new/separate one. |
7193ae9
to
b648845
Compare
Is resume fixed with this? Currently it starts overriding old data (because step begins from 0 after resuming I think). |
@IgnacioFDM , yes! That should fix this. Previously, we set |
Just tested it and each time you resume, it creates a new run (with the same name) on wandb. |
@tcapelle , thanks for your code snippet a while back and sorry for this late PR. Following your feedback to use |
Ohh great! Yeah, for resuming, you need the id, so you would need to keep both attributed. Sorry for missing that. |
I tried setting the WANDB_RUN_ID and it works like it did before this PR: it continues the run, but still leads to wrong data (either new steps don't show up, or they show up but it starts deleting earlier steps). I think the issue arises from the fact that step (not global_step) resets to 0 when you resume. |
b648845
to
bfbaa88
Compare
Rebased! Updated it to support both |
* Feat: Update to handle wandb env better * chore: rename wandb_run_id to wandb_name * feat: add new recommendation and update config * fix: indent and pop disabled env if project passed * feat: test env set for wandb and recommendation * feat: update to use wandb_name and allow id * chore: add info to readme
* Feat: Update to handle wandb env better * chore: rename wandb_run_id to wandb_name * feat: add new recommendation and update config * fix: indent and pop disabled env if project passed * feat: test env set for wandb and recommendation * feat: update to use wandb_name and allow id * chore: add info to readme
Closes #236
Breaking change:
wandb_run_id
->wandb_name
following wandb eng's recommendation Improve documentation and implementation of W&B options #236 (comment) . I have added a warning and backward compatible fix for now.WANDB_DISABLED
env ifwandb_project
passed. This should be the expected behavior.New feature:
wandb_
config to be passed to env for more flexibility