-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Add Weights and Biases Logger #427
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
Conversation
better to keep BasicLogger for compatibility, I'll fix that |
I find BaseLogger BasicLogger naming confusing a bit. But maybe we can do it in another release or something. |
I know, I mean class BasicLogger(TensorboardLogger):
def __init__(self, *args, **kwargs):
warnings.warn("better to use TensorboardLogger")
super().__init__(*args, **kwargs) |
Oooh i see, good idea! Also, for this I usually see people put something like "Deprecated: use TensorboardLogger instead" |
In [1]: from tianshou.utils import BasicLogger
In [2]: a=BasicLogger(None)
/home/trinkle/github/tianshou-new/tianshou/utils/logger/tensorboard.py:135: UserWarning: Deprecated soon: BasicLogger has renamed to TensorboardLogger in #427.
warnings.warn( |
I'm refactoring this part now. |
2. Use ``BasicLogger`` which contains a tensorboard; | ||
3. To adjust the save frequency, specify ``save_interval`` when initializing BasicLogger. | ||
2. Use ``TensorboardLogger``; | ||
3. To adjust the save frequency, specify ``save_interval`` when initializing TensorboardLogger. |
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.
Is it possible to resume an experiment with wandb? like #350
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.
I think so, but I've never dealt with it before. I suggest waiting for people who have that specific need to raise an issue.
Because I think to resume it you need to start it differently - and it may be extra complexity not everyone needs.
@@ -22,7 +22,7 @@ class PPOPolicy(A2CPolicy): | |||
:param float dual_clip: a parameter c mentioned in arXiv:1912.09729 Equ. 5, | |||
where c > 1 is a constant indicating the lower bound. | |||
Default to 5.0 (set None if you do not want to use it). | |||
:param bool value_clip: a parameter mentioned in arXiv:1811.02553 Sec. 4.1. | |||
:param bool value_clip: a parameter mentioned in arXiv:1811.02553v3 Sec. 4.1. |
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.
Codecov Report
@@ Coverage Diff @@
## master #427 +/- ##
==========================================
- Coverage 94.82% 94.68% -0.14%
==========================================
Files 58 60 +2
Lines 3807 3820 +13
==========================================
+ Hits 3610 3617 +7
- Misses 197 203 +6
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Should I add wandb logger test such as https://wandb.ai/tianshou/tianshou-new-test_continuous/runs/3gcpla04?workspace=user-tianshou? |
Those runs are not visible to me. |
Okay so plz review my change |
Looks good to me, I can start using to make sure everything works. |
- fix a bug in #427: logger.write should pass a dict - change SubprocVectorEnv to ShmemVectorEnv in atari - increase logger interval for eps
- rename BasicLogger to TensorboardLogger - refactor logger code - add WandbLogger Co-authored-by: Jiayi Weng <trinkle23897@gmail.com>
- fix a bug in thu-ml#427: logger.write should pass a dict - change SubprocVectorEnv to ShmemVectorEnv in atari - increase logger interval for eps
This fixes #426.
Summary of changes
utils/__init__.py
please.P.S.: As a side note, the unit tests take a really long time to run.