Remove dummy net code #123

youkaichao · 2020-07-09T11:14:00Z

I have marked all applicable categories:
- exception-raising fix
- algorithm implementation fix
- documentation modification
- new feature
If applicable, I have mentioned the relevant/related issue(s)

Less important but also useful:

I have visited the source website, and in particular read the known issues
I have searched through the issue tracker and issue categories for duplicates

I have mentioned version numbers, operating system and environment, where applicable:

import tianshou, torch, sys
print(tianshou.__version__, torch.__version__, sys.version, sys.platform)

codecov-commenter · 2020-07-09T11:55:17Z

Codecov Report

Merging #123 into master will decrease coverage by 1.43%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #123      +/-   ##
==========================================
- Coverage   84.94%   83.50%   -1.44%     
==========================================
  Files          29       32       +3     
  Lines        1906     2092     +186     
==========================================
+ Hits         1619     1747     +128     
- Misses        287      345      +58

Flag	Coverage Δ
#unittests	`83.50% <100.00%> (-1.44%)`	⬇️

Impacted Files	Coverage Δ
tianshou/utils/net/common.py	`100.00% <100.00%> (ø)`
tianshou/utils/net/continuous.py	`61.11% <100.00%> (ø)`
tianshou/utils/net/discrete.py	`52.08% <100.00%> (ø)`

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 aa3c453...e415320. Read the comment docs.

duburcqa · 2020-07-09T11:59:09Z

Why not to keep the original names Actor / Critic? I think it is clearer. I don't see any advantage of introducing extra postfix Head. It is not confusing since Net is clearly not an Actor or Critic on its own.

youkaichao · 2020-07-09T12:25:22Z

Why not to keep the original names Actor / Critic? I think it is clearer. I don't see any advantage of introducing extra postfix Head. It is not confusing since Net is clearly not an Actor or Critic on its own.

Good idea, solved in new commit.

examples/ant_v2_ddpg.py

examples/ant_v2_sac.py

examples/ant_v2_td3.py

examples/halfcheetahBullet_v0_sac.py

…stration

Trinkle23897 · 2020-10-31T07:55:21Z

examples/ant_v2_td3.py

+    net = Net(args.layer_num, args.state_shape, device=args.device)
    actor = Actor(
-        args.layer_num, args.state_shape, args.action_shape,
+        net, args.action_shape,
        args.max_action, args.device
    ).to(args.device)
    actor_optim = torch.optim.Adam(actor.parameters(), lr=args.actor_lr)
-    critic1 = Critic(
-        args.layer_num, args.state_shape, args.action_shape, args.device
-    ).to(args.device)
+    net = Net(args.layer_num, args.state_shape,
+              args.action_shape, concat=True, device=args.device)
+    critic1 = Critic(net, args.device).to(args.device)
    critic1_optim = torch.optim.Adam(critic1.parameters(), lr=args.critic_lr)
-    critic2 = Critic(
-        args.layer_num, args.state_shape, args.action_shape, args.device
-    ).to(args.device)
+    critic2 = Critic(net, args.device).to(args.device)


Actually, TD3 cannot support share-critic, which causes #209.

* remove dummy net; delete two files * split code to have backbone and head * rename class * change torch.float to torch.float32 * use flatten(1) instead of view(batch, -1) * remove dummy net in docs * bugfix for rnn * fix cuda error * minor fix of docs * do not change the example code in dqn tutorial, since it is for demonstration Co-authored-by: Trinkle23897 <463003665@qq.com>

youkaichao added 2 commits July 9, 2020 17:30

remove dummy net; delete two files

ba87cb4

split code to have backbone and head

e415320

Trinkle23897 mentioned this pull request Jul 9, 2020

Add multi-agent example: tic-tac-toe #122

Merged

rename class

277fb4e

change torch.float to torch.float32

185f5dc

duburcqa approved these changes Jul 9, 2020

View reviewed changes

youkaichao added 3 commits July 9, 2020 21:07

use flatten(1) instead of view(batch, -1)

ba7c184

remove dummy net in docs

1474a6c

bugfix for rnn

6d0f9af

Trinkle23897 reviewed Jul 9, 2020

View reviewed changes

examples/ant_v2_ddpg.py Outdated Show resolved Hide resolved

examples/ant_v2_sac.py Outdated Show resolved Hide resolved

examples/ant_v2_td3.py Outdated Show resolved Hide resolved

examples/halfcheetahBullet_v0_sac.py Outdated Show resolved Hide resolved

Trinkle23897 added 4 commits July 9, 2020 22:20

fix cuda error

cd722c8

minor fix of focs

b9a314e

minor fix of docs

8e8ec66

do not change the example code in dqn tutorial, since it is for demon…

b507bb5

…stration

Trinkle23897 approved these changes Jul 9, 2020

View reviewed changes

Trinkle23897 merged commit e767de0 into thu-ml:master Jul 9, 2020

youkaichao deleted the remove_dummy_net branch July 9, 2020 15:04

Trinkle23897 reviewed Oct 31, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Remove dummy net code #123

Remove dummy net code #123

Uh oh!

youkaichao commented Jul 9, 2020

Uh oh!

codecov-commenter commented Jul 9, 2020

Uh oh!

duburcqa commented Jul 9, 2020

Uh oh!

youkaichao commented Jul 9, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Trinkle23897 Oct 31, 2020

Uh oh!

Uh oh!

Remove dummy net code #123

Remove dummy net code #123

Uh oh!

Conversation

youkaichao commented Jul 9, 2020

Uh oh!

codecov-commenter commented Jul 9, 2020

Codecov Report

Uh oh!

duburcqa commented Jul 9, 2020

Uh oh!

youkaichao commented Jul 9, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Trinkle23897 Oct 31, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!