Distributed Framework by threewisemonkeys-as · Pull Request #327 · SforAiDl/genrl

threewisemonkeys-as · 2020-09-03T18:10:00Z

This is a very rough draft of a trainer for distributed off policy agents.

Currently working on getting DDPG to be trained in distributed manner using this.

codecov · 2020-09-03T18:26:16Z

Codecov Report

❌ Patch coverage is 7.07547% with 197 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.83%. Comparing base (c3ef4d9) to head (8030b2a).
⚠️ Report is 34 commits behind head on master.

Files with missing lines	Patch %	Lines
genrl/distributed/core.py	0.00%	97 Missing ⚠️
genrl/distributed/parameter_server.py	0.00%	22 Missing ⚠️
genrl/trainers/distributed.py	29.62%	19 Missing ⚠️
genrl/distributed/actor.py	0.00%	18 Missing ⚠️
genrl/distributed/learner.py	0.00%	18 Missing ⚠️
genrl/distributed/experience_server.py	0.00%	14 Missing ⚠️
genrl/distributed/__init__.py	0.00%	5 Missing ⚠️
genrl/agents/deep/base/offpolicy.py	66.66%	2 Missing ⚠️
genrl/core/buffers.py	33.33%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #327      +/-   ##
==========================================
- Coverage   90.87%   86.83%   -4.05%     
==========================================
  Files          88       97       +9     
  Lines        3705     4017     +312     
==========================================
+ Hits         3367     3488     +121     
- Misses        338      529     +191

Files with missing lines	Coverage Δ
genrl/agents/deep/ddpg/ddpg.py	`86.20% <100.00%> (-6.11%)`	⬇️
genrl/trainers/__init__.py	`100.00% <100.00%> (ø)`
genrl/agents/deep/base/offpolicy.py	`96.20% <66.66%> (-1.21%)`	⬇️
genrl/core/buffers.py	`93.84% <33.33%> (-1.40%)`	⬇️
genrl/distributed/__init__.py	`0.00% <0.00%> (ø)`
genrl/distributed/experience_server.py	`0.00% <0.00%> (ø)`
genrl/distributed/actor.py	`0.00% <0.00%> (ø)`
genrl/distributed/learner.py	`0.00% <0.00%> (ø)`
genrl/trainers/distributed.py	`29.62% <29.62%> (ø)`
genrl/distributed/parameter_server.py	`0.00% <0.00%> (ø)`
... and 1 more

... and 23 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

lgtm-com · 2020-09-03T18:38:54Z

This pull request introduces 3 alerts when merging 4cba727 into 0fe4180 - view on LGTM.com

new alerts:

2 for Unused local variable
1 for Unused import

Sharad24

Have you tried it on any agent yet?

lgtm-com · 2020-09-06T18:56:35Z

This pull request introduces 10 alerts when merging 3d233c4 into 0fe4180 - view on LGTM.com

new alerts:

9 for Unused import
1 for Unused local variable

lgtm-com · 2020-09-20T14:33:36Z

This pull request introduces 15 alerts when merging 1c504cc into bb85ea1 - view on LGTM.com

new alerts:

14 for Unused import
1 for Unused local variable

lgtm-com · 2020-10-01T13:04:42Z

This pull request introduces 19 alerts when merging 2c3298a into 608fc03 - view on LGTM.com

new alerts:

14 for Unused import
2 for Unused local variable
2 for Wrong number of arguments in a call
1 for Missing call to __init__ during object initialization

lgtm-com · 2020-10-07T14:03:38Z

This pull request introduces 14 alerts when merging 73586d5 into 52b0b4c - view on LGTM.com

new alerts:

13 for Unused import
1 for Unused local variable

lgtm-com · 2020-10-07T14:42:02Z

This pull request introduces 2 alerts when merging 072d545 into 52b0b4c - view on LGTM.com

new alerts:

1 for Signature mismatch in overriding method
1 for Unused import

Sharad24 · 2020-10-08T08:03:01Z

+class WeightHolder:
+    def __init__(self, init_weights):
+        self._weights = init_weights


nit
You could add a decorator @dataclass to avoid __init__

Same for other classes where there's only assigning business happening in the constructor

Sharad24 · 2020-10-08T08:08:11Z

+        learner_rref = get_rref(learner_name)
+        print(f"{name}: Begining experience collection")
+        while not learner_rref.rpc_sync().is_done():
+            agent.load_weights(parameter_server_rref.rpc_sync().get_weights())


Would it be better to assign the agent in the constructor itself?

Assign agent weights? They will need to be updated in the loop right?

Sharad24 · 2020-10-08T08:14:59Z

+def collect_experience(agent, experience_server_rref):
+    obs = agent.env.reset()
+    done = False
+    for i in range(MAX_ENV_STEPS):
+        action = agent.select_action(obs)
+        next_obs, reward, done, info = agent.env.step(action)
+        experience_server_rref.rpc_sync().push((obs, action, reward, next_obs, done))
+        obs = next_obs
+        if done:
+            break


This experience collection is working only on a single agent/single thread?

This is being run in multiple different processes. Its being passed to the ActorNode which is running it in its own infinite loop

Sharad24

How is the Actor definition going to work? Can I define any architecture for the actor?(this would be ideal behavior)

(I dont see any neural network definitions as of now).

Another thought that I had was: do you think we could somehow use decorators here? There's a bunch of core details we can get rid of then.

lgtm-com · 2020-10-23T13:38:52Z

This pull request introduces 2 alerts when merging e2eef66 into 25eb018 - view on LGTM.com

new alerts:

2 for Unused import

lgtm-com · 2020-10-23T18:24:17Z

This pull request introduces 2 alerts when merging 837eb18 into 25eb018 - view on LGTM.com

new alerts:

2 for Unused import

threewisemonkeys-as · 2020-10-29T18:00:47Z

How is the Actor definition going to work? Can I define any architecture for the actor?(this would be ideal behavior)

Yeah. The ActorNode class only has multiprocessing details, it can basically accommodate any agent. You can see an example in the onpolicy example. The user has the ability to define which agent to run through the agent argument to the ActorNode and how to run it using the collect_experience function also passed as argument.

threewisemonkeys-as · 2020-10-29T18:03:33Z

(I dont see any neural network definitions as of now).

This is happening internally in the DDPG agent I'm passing to the ActorNode. For On policy the current genrl framework was too inflexible. So I created my own in the onpolicy example. I think we should move towards this kind of more modular structure in general instead of the current one which is more hierarchical.

lgtm-com · 2020-10-29T18:18:15Z

This pull request introduces 5 alerts when merging 8030b2a into 25eb018 - view on LGTM.com

new alerts:

5 for Unused import

threewisemonkeys-as · 2020-10-29T18:52:23Z

Another thought that I had was: do you think we could somehow use decorators here? There's a bunch of core details we can get rid of then.

I haven't used decorators too extensively before, I'll look into it though. Did you have any specific ideas in mind?

threewisemonkeys-as added 4 commits August 28, 2020 01:17

added demo notebooks

2567fe5

Merge branch 'master' of https://github.com/SforAiDl/genrl

1aebe3c

Merge branch 'master' of https://github.com/SforAiDl/genrl

e3b8a8a

initial structure

4cba727

Sharad24 reviewed Sep 4, 2020

View reviewed changes

added mp

3d233c4

add files

1c504cc

added new structure on rpc

2c3298a

threewisemonkeys-as added 3 commits October 7, 2020 13:34

working structure

73586d5

fixed integration bugs

9ef6845

removed unneccary files

072d545

threewisemonkeys-as added 2 commits October 8, 2020 04:18

added support for running from multiple scripts

64db1c1

added evaluate to trainer

4d57a06

Sharad24 reviewed Oct 8, 2020

View reviewed changes

threewisemonkeys-as added 6 commits October 23, 2020 06:45

added proxy getter

f325429

added rpc backend option

7ce19ec

added logging to trainer

cfba909

Added more options to trainer

992a3a9

moved load weights to user

bf1a50a

decreased number of eval its

e2eef66

removed train wrapper

837eb18

threewisemonkeys-as changed the title ~~Adding distributed trainer~~ Distributed Framework Oct 25, 2020

threewisemonkeys-as added 10 commits October 26, 2020 12:06

removed loop to user fn

7fcbb23

added example for secondary node

0002fa4

removed original exmpale

bebf50f

removed fn

29bd1d6

shifted examples

18536a2

shifted logger to base class

8f859d6

added on policy example

555e290

removed temp example

59e960c

got on policy distributed example to work

8d5a8b6

formatting

8030b2a

threewisemonkeys-as linked an issue Nov 4, 2020 that may be closed by this pull request

RPC Communication in Distributed RL Training #303

Open

Uh oh!

Conversation

threewisemonkeys-as commented Sep 3, 2020

Uh oh!

codecov Bot commented Sep 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

lgtm-com Bot commented Sep 3, 2020

Uh oh!

Sharad24 left a comment

Choose a reason for hiding this comment

Uh oh!

lgtm-com Bot commented Sep 6, 2020

Uh oh!

lgtm-com Bot commented Sep 20, 2020

Uh oh!

lgtm-com Bot commented Oct 1, 2020

Uh oh!

lgtm-com Bot commented Oct 7, 2020

Uh oh!

lgtm-com Bot commented Oct 7, 2020

Uh oh!

Sharad24 Oct 8, 2020

Choose a reason for hiding this comment

Uh oh!

Sharad24 Oct 8, 2020

Choose a reason for hiding this comment

Uh oh!

Sharad24 Oct 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

threewisemonkeys-as Oct 29, 2020

Choose a reason for hiding this comment

Uh oh!

Sharad24 Oct 8, 2020

Choose a reason for hiding this comment

Uh oh!

threewisemonkeys-as Oct 29, 2020

Choose a reason for hiding this comment

Uh oh!

Sharad24 left a comment

Choose a reason for hiding this comment

Uh oh!

lgtm-com Bot commented Oct 23, 2020

Uh oh!

lgtm-com Bot commented Oct 23, 2020

Uh oh!

threewisemonkeys-as commented Oct 29, 2020

Uh oh!

threewisemonkeys-as commented Oct 29, 2020

Uh oh!

lgtm-com Bot commented Oct 29, 2020

Uh oh!

threewisemonkeys-as commented Oct 29, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov Bot commented Sep 3, 2020 •

edited

Loading

Sharad24 Oct 8, 2020 •

edited

Loading