W&B: add artifacts support #441

AyushExel · 2021-09-07T08:54:40Z

I have marked all applicable categories:
- exception-raising fix
- algorithm implementation fix
- documentation modification
- new feature
I have reformatted the code using make format (required)
I have checked the code using make commit-checks (required)
If applicable, I have mentioned the relevant/related issue(s)
If applicable, I have listed every items in this Pull Request below

This PR fixes existing bugs in WandbLogger and adds support for artifacts.

When using WandbLogger, you can now resume your runs from any device. Example usage is in the examples/atari/atari_dqn_wandb.py:

cd examples/atari
python atari_dqn_wandb.py
# terminate run. The run is executable on any device via
python atari_dqn_wandb.py --resume_id {your run id}

Let me know if I missed something. This is still a WIP. I'll add some more advanced visualization features.

Bug fix

It seems like the atari_dqn task is passing floats to logger.write method which crashes the script. I've handled the use case but it is hacky. We might need to modify the trainer to always pass dicts

tianshou/utils/logger/wandb.py

Trinkle23897 · 2021-09-07T12:19:26Z

examples/atari/atari_dqn_wandb.py

+        else:
+            eps = args.eps_train_final
+        policy.set_eps(eps)
+        logger.write('train/eps', env_step, eps)


this is the root course of that bug, I'll fix that

codecov-commenter · 2021-09-07T12:20:00Z

Codecov Report

Merging #441 (dac8958) into master (e8f8cdf) will decrease coverage by 0.50%.
The diff coverage is 38.23%.

@@            Coverage Diff             @@
##           master     #441      +/-   ##
==========================================
- Coverage   94.71%   94.21%   -0.51%     
==========================================
  Files          60       60              
  Lines        3821     3853      +32     
==========================================
+ Hits         3619     3630      +11     
- Misses        202      223      +21

Flag	Coverage Δ
unittests	`94.21% <38.23%> (-0.51%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
tianshou/utils/logger/wandb.py	`47.72% <36.36%> (-10.61%)`	⬇️
tianshou/utils/__init__.py	`100.00% <100.00%> (ø)`
tianshou/utils/logger/base.py	`91.66% <0.00%> (-4.17%)`	⬇️
tianshou/policy/modelfree/npg.py	`97.70% <0.00%> (-1.15%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e8f8cdf...dac8958. Read the comment docs.

Trinkle23897 · 2021-09-07T12:20:51Z

I'm wondering if we can add a unit test of wandb under test/base/, what do you think?

AyushExel · 2021-09-07T12:34:39Z

I'm wondering if we can add a unit test of wandb under test/base/, what do you think?

@Trinkle23897 yes I'll do that before marking this ready for review. I think the easiest way to add tests would be to set up a dummy account that we can use for CI tests.

Co-authored-by: Jiayi Weng <trinkle23897@qq.com>

Trinkle23897 · 2021-09-07T12:47:18Z

I've already set up https://wandb.ai/tianshou a week before, maybe we can use a sub-namespace under this account?

AyushExel · 2021-09-07T12:55:45Z

I've already set up https://wandb.ai/tianshou a week before, maybe we can use a sub-namespace under this account?

Okay. One thing to note here would be that the API key of the account used for CI will be exposed. So, it'll be better to use an account that doesn't have any important experiments.

Trinkle23897 · 2021-09-07T12:58:56Z

But I think we can store the API key inside environment secrets:

AyushExel · 2021-09-07T13:01:29Z

@Trinkle23897 Ohh I wasn't aware of that. Yes, that would work!

Trinkle23897 · 2021-09-07T13:03:46Z

Just configure secret as follows:

let me know if you need further changes (also you can experiment this feature in your own fork)

AyushExel · 2021-09-07T13:05:27Z

This is very helpful. Thanks!

Trinkle23897 · 2021-09-07T13:06:37Z

my bad. should be this one:

AyushExel · 2021-09-08T05:33:15Z

@Trinkle23897 I was trying to log gym visualizations over time to show training progress something like this. To do that we need to monitor the videos from the gym. So I added env = gym.wrappers.Monitor(env, "videos", force=True) after this line . But now it throws an error:

Traceback (most recent call last):
  File "/opt/conda/lib/python3.7/multiprocessing/process.py", line 297, in _bootstrap
    self.run()
  File "/opt/conda/lib/python3.7/multiprocessing/process.py", line 99, in run
    self._target(*self._args, **self._kwargs)
  File "/home/jupyter/repos/tianshou/tianshou/env/worker/subproc.py", line 95, in _worker
    obs = env.reset()
  File "/home/jupyter/repos/tianshou/examples/atari/atari_wrapper.py", line 196, in reset
    obs = self.env.reset()
  File "/opt/conda/lib/python3.7/site-packages/gym/core.py", line 278, in reset
    observation = self.env.reset(**kwargs)
  File "/home/jupyter/repos/tianshou/examples/atari/atari_wrapper.py", line 115, in reset
    self.env.reset()
  File "/opt/conda/lib/python3.7/site-packages/gym/core.py", line 251, in reset
    return self.env.reset(**kwargs)
  File "/home/jupyter/repos/tianshou/examples/atari/atari_wrapper.py", line 26, in reset
    self.env.reset()
  File "/opt/conda/lib/python3.7/site-packages/gym/wrappers/monitor.py", line 52, in reset
    self._before_reset()
  File "/opt/conda/lib/python3.7/site-packages/gym/wrappers/monitor.py", line 230, in _before_reset
    self.stats_recorder.before_reset()
  File "/opt/conda/lib/python3.7/site-packages/gym/wrappers/monitoring/stats_recorder.py", line 82, in before_reset
    self.env_id
gym.error.Error: Tried to reset environment which is not done. While the monitor is active for PongNoFrameskip-v4, you cannot call reset() unless the episode is over.
Traceback (most recent call last):
  File "atari_dqn_wandb.py", line 216, in <module>
    test_dqn(get_args())
  File "atari_dqn_wandb.py", line 208, in test_dqn
    save_checkpoint_fn=save_checkpoint_fn
  File "/home/jupyter/repos/tianshou/tianshou/trainer/offpolicy.py", line 97, in offpolicy_trainer
    env_step, reward_metric
  File "/home/jupyter/repos/tianshou/tianshou/trainer/utils.py", line 22, in test_episode
    collector.reset_env()
  File "/home/jupyter/repos/tianshou/tianshou/data/collector.py", line 118, in reset_env
    obs = self.env.reset()
  File "/home/jupyter/repos/tianshou/tianshou/env/venvs.py", line 173, in reset
    obs_list = [self.workers[i].reset() for i in id]
  File "/home/jupyter/repos/tianshou/tianshou/env/venvs.py", line 173, in <listcomp>
    obs_list = [self.workers[i].reset() for i in id]
  File "/home/jupyter/repos/tianshou/tianshou/env/worker/subproc.py", line 165, in reset
    obs = self.parent_remote.recv()
  File "/opt/conda/lib/python3.7/multiprocessing/connection.py", line 250, in recv
    buf = self._recv_bytes()
  File "/opt/conda/lib/python3.7/multiprocessing/connection.py", line 407, in _recv_bytes
    buf = self._recv(4)
  File "/opt/conda/lib/python3.7/multiprocessing/connection.py", line 383, in _recv
    raise EOFError
EOFError

Trinkle23897 · 2021-09-08T15:07:30Z

it may because env.reset() after env.reset() inside the initialization process. a quick fix is to add another wrapper that can handle this consecutive reset into a single one

drozzy

I would do it, but I don't really know how to modify an in-progress PR.

tianshou/utils/logger/wandb.py

AyushExel · 2021-09-16T14:06:16Z

@Trinkle23897 @drozzy the psrl test is failing. I think the latest gym doesn't include gym_toytext. I installed it using pip install gym-legacy-toytext and imported it using import gym_toytext and the psrl test worked. See the logging test

Trinkle23897 · 2021-09-16T14:13:24Z

how about DLR-RM/stable-baselines3@e7a48d2#diff-60f61ab7a8d1910d86d9fda2261620314edcae5894d5aaa236b821c7256badd7R76

AyushExel · 2021-09-16T14:20:51Z

@Trinkle23897 ok I've updated the gym req. based on that link

AyushExel · 2021-09-16T14:51:20Z

@Trinkle23897 Do you know how the github env secret can be accessed in python script? For the wandb test to pass we need to login in the script like wandb.login(api_key=secret_api_key)

Trinkle23897 · 2021-09-16T14:53:25Z

wandb.login(api_key=os.environ["WANDB_API_KEY"])

AyushExel · 2021-09-20T18:42:41Z

@Trinkle23897 Ok now it's working

AyushExel · 2021-09-21T08:24:48Z

@Trinkle23897 Hi. This is ready to be merged from my end. Do you have any changes that you'd like me to address.
Once this is merged, I can start with updating docs and a getting started colab

Trinkle23897 · 2021-09-21T11:49:12Z

Could you please wait for me to finish one assignment for 2 days? I'll polish it.

AyushExel · 2021-09-21T13:34:17Z

@Trinkle23897 sure. Take your time :)

AyushExel · 2021-09-23T12:15:22Z

@Trinkle23897 Would you like to add the instructions on how to use this logger in the docs or readme?

Trinkle23897 · 2021-09-23T13:44:19Z

yep

AyushExel · 2021-09-23T16:32:52Z

Awesome

tianshou/utils/logger/wandb.py

Trinkle23897 · 2021-09-23T20:30:27Z

test/modelbased/test_psrl.py

+    if args.logger == "wandb":
+        logger = WandbLogger(save_interval=1, project='psrl', name='wandb_test')
+    elif args.logger == "tensorboard":
+        log_path = os.path.join(args.logdir, args.task, 'psrl')
+        writer = SummaryWriter(log_path)
+        writer.add_text("args", str(args))
+        logger = TensorboardLogger(writer)


Is it possible to log all args into wandb at beginning?

yeah. I'll add it now

Okay. I've added configuration logging. Hopefully tests succeed

Trinkle23897 · 2021-09-23T21:12:13Z

tianshou/utils/logger/wandb.py


-    Creates three panels with plots: train, test, and update.
+    This logger creates three panels with plots: train, test, and update.
    Make sure to select the correct access for each panel in weights and biases:



maybe we can here show where the example script is?

Trinkle23897 · 2021-09-23T21:13:17Z

examples/atari/atari_dqn.py

@@ -41,6 +41,13 @@ def get_args():
    )
    parser.add_argument('--frames-stack', type=int, default=4)
    parser.add_argument('--resume-path', type=str, default=None)
+    parser.add_argument('--resume-id', type=str, default=None)
+    parser.add_argument(
+        '--logger',


and maybe we can add some instructions on how to use wandb (including resume) in examples/atari/README.md instead of tensorboard?

Can we add the instructions in doc also? I can make a separate PR tomorrow for adding the detailed instructions in examples/atari/README.md as well as docs.

Just append to this pr

Trinkle23897 · 2021-09-23T21:16:48Z

Great work @AyushExel, please check if my modification is correct or not (I haven't run the atari_dqn script for resume training and see if wandb still work). I change loading checkpoint under args.resume_id to args.resume_path because users can specify which model to load. This is a trade-off: it may cause inconsistency when loading model and logger, but gives more flexibility when they don't want to resume logger (this is the common case).

AyushExel · 2021-09-23T21:23:59Z

@Trinkle23897 Thanks. Yeah It makes sense. I'm testing the resume functionality on atari now.
Edit: It's working :)

AyushExel · 2021-09-24T05:59:59Z

@Trinkle23897 alright I think this is good to go. I can add instructions and details in a separate PR.

Trinkle23897 · 2021-09-24T13:49:09Z

sure!

- rename WandBLogger -> WandbLogger - add save_data and restore_data - allow more input arguments for wandb init - integrate wandb into test/modelbase/test_psrl.py and examples/atari/atari_dqn.py - documentation update

AyushExel added 2 commits September 7, 2021 08:38

add artifacts support

3a46660

remove print

32b3507

Trinkle23897 reviewed Sep 7, 2021

View reviewed changes

Update tianshou/utils/logger/wandb.py

1cc306e

Co-authored-by: Jiayi Weng <trinkle23897@qq.com>

AyushExel added 2 commits September 8, 2021 14:53

monitor gym

5cc42b4

monitor gym

5617fe2

Merge branch 'master' into wandb

c751375

drozzy reviewed Sep 13, 2021

View reviewed changes

tianshou/utils/logger/wandb.py Show resolved Hide resolved

AyushExel added 3 commits September 15, 2021 14:46

Update logger

94de701

repo label

0ac506c

add test

00e4afb

AyushExel marked this pull request as ready for review September 16, 2021 14:06

update gym req.

e8616ff

try ci fix

3c09be7

Trinkle23897 changed the title ~~[WIP] W&B: add artifacts support~~ W&B: add artifacts support Sep 23, 2021

update docs

0931a08

Trinkle23897 reviewed Sep 23, 2021

View reviewed changes

tianshou/utils/logger/wandb.py Outdated Show resolved Hide resolved

AyushExel and others added 4 commits September 24, 2021 01:54

Update wandb.py

0bd9134

unify logger test on psrl

261ee46

Merge branch 'wandb' of github.com:AyushExel/tianshou into wandb

8d7e423

Update wandb.py

b1698c0

Trinkle23897 reviewed Sep 23, 2021

View reviewed changes

Trinkle23897 and others added 6 commits September 23, 2021 16:34

fix format

684afbb

add config logging

ebf6015

Merge branch 'wandb' of https://github.com/AyushExel/tianshou into wandb

0853f6b

update

397de8a

merge atari_wandb into original file

1635000

fix format

dac8958

Trinkle23897 reviewed Sep 23, 2021

View reviewed changes

Trinkle23897 approved these changes Sep 24, 2021

View reviewed changes

Trinkle23897 merged commit 22d7bf3 into thu-ml:master Sep 24, 2021

W&B: add artifacts support #441

W&B: add artifacts support #441

Uh oh!

Conversation

AyushExel commented Sep 7, 2021 • edited by Trinkle23897 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Bug fix

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Sep 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Trinkle23897 commented Sep 7, 2021

Uh oh!

AyushExel commented Sep 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Trinkle23897 commented Sep 7, 2021

Uh oh!

AyushExel commented Sep 7, 2021

Uh oh!

Trinkle23897 commented Sep 7, 2021

Uh oh!

AyushExel commented Sep 7, 2021

Uh oh!

Trinkle23897 commented Sep 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AyushExel commented Sep 7, 2021

Uh oh!

Trinkle23897 commented Sep 7, 2021

Uh oh!

AyushExel commented Sep 8, 2021

Uh oh!

Trinkle23897 commented Sep 8, 2021

Uh oh!

drozzy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

AyushExel commented Sep 16, 2021

Uh oh!

Trinkle23897 commented Sep 16, 2021

Uh oh!

AyushExel commented Sep 16, 2021

Uh oh!

AyushExel commented Sep 16, 2021

Uh oh!

Trinkle23897 commented Sep 16, 2021

Uh oh!

AyushExel commented Sep 20, 2021

Uh oh!

AyushExel commented Sep 21, 2021

Uh oh!

Trinkle23897 commented Sep 21, 2021

Uh oh!

AyushExel commented Sep 21, 2021

Uh oh!

AyushExel commented Sep 23, 2021

Uh oh!

Trinkle23897 commented Sep 23, 2021

Uh oh!

AyushExel commented Sep 23, 2021

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AyushExel commented Sep 7, 2021 •

edited by Trinkle23897

Loading

codecov-commenter commented Sep 7, 2021 •

edited

Loading

AyushExel commented Sep 7, 2021 •

edited

Loading

Trinkle23897 commented Sep 7, 2021 •

edited

Loading

Trinkle23897 commented Sep 23, 2021 •

edited

Loading

AyushExel commented Sep 23, 2021 •

edited

Loading