-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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): let wandb cli handle runs #4648
Conversation
This should not change how the callback is used. |
Hello @borisdayma! Thanks for updating this PR. There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2020-11-23 18:46:05 UTC |
Co-authored-by: Jirka Borovec <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #4648 +/- ##
=======================================
Coverage 93% 93%
=======================================
Files 118 118
Lines 8910 9020 +110
=======================================
+ Hits 8284 8393 +109
- Misses 626 627 +1 |
@@ -126,7 +128,7 @@ def experiment(self) -> Run: | |||
os.environ['WANDB_MODE'] = 'dryrun' | |||
self._experiment = wandb.init( |
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.
Should we handle legacy there ?
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.
That's a good question. The new wandb CLI appeared in v0.10.0 (9/11/2020). However the reinit
logic has actually changed for several months (though it's probably the most robust in the latest versions).
Using the latest version of wandb
should not be a breaking change since reinit
was forced to True
and now it will automatically do what should be expected (ie start a run only when none is existing). Also it's now directly accessible through environment variables if users needed to set it to True for some reasons.
I added a test to check |
Hi guys, just checking if you need anything else from me |
@borisdayma I'm currently checking why tests are failing, that's why I removed the ready label. |
good catch @rohitgr7 thanks!! |
What does this PR do?
With the release of wandb version >.0.10, the logic for
reinit
should be handled directly bywandb
instead of forcing it to beTrue
in the logger. This allows better handling of runs in jupyter/colab.In addition, a new run should not be created if it is already existing as it creates unexpected behavior (automatically closing a run manually launched or run by a sweep).
If a user needs to create multiple runs in the same script, he can now just call
wandb.finish()
at any moment (which was not available previously).Before submitting
PR review
Anyone in the community is free to review the PR once the tests have passed.
Before you start reviewing make sure you have read Review guidelines. In in short, see following bullet-list:
Did you have fun?
Make sure you had fun coding 🙃