[rllib] ExternalMultiAgentEnv #4200

ctombumila37 · 2019-02-28T14:39:38Z

What do these changes do?

Create a combination of ExternalEnv and MultiAgentEnv, called ExternalMutliAgentEnv

Related issue number

#4051

Please note that this PR is by far not finished.

For things that do not work (yet), see the commit messages.

I am a novice in ray / rllib, thus I would appreciate any help with this :)

To Do

move the functionality of _ExternalMultiAgentEnvEpisode into _ExternalEnvEpisode
add a unit test in test_external_env that exercises multi-agent external envs

- self.cur_reward in _ExternalEnvEpisode for some reason has to know the ids of all possible agents (serving multiagent example) - the thread of ExternalMultiAgentEnv gets started multiple times, thus ppo.train() deadlocks in the example

AmplabJenkins · 2019-02-28T17:39:36Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/12391/
Test FAILed.

ericl · 2019-03-01T02:36:51Z

Hey, thanks for getting started on this. One idea: is it possible to add multi-agent to the existing external env adapter in a backwards compatible way? For example, adding option arguments to specify an agent id. That way, there could be less code duplication between the two envs.

introduce a flag 'multiagent' in _ExternalEnvToBaseEnv to reduce code duplication.

AmplabJenkins · 2019-03-01T11:10:27Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/12417/
Test FAILed.

AmplabJenkins · 2019-03-01T13:36:37Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/12425/
Test FAILed.

ericl · 2019-03-03T06:44:00Z

python/ray/rllib/env/external_multi_agent_env.py

+        return episode_id
+
+    @PublicAPI
+    def get_action(self, episode_id, observation):


One design decision here is whether you should be getting the action of multiple agents at once (as opposed to one at a time).

I think it may be easier to enforce all the observations of all agents acting for the "episode step" must be provided at once in this call. Otherwise, it becomes unclear which agent actions go in which episode timestep.

While I think it would be easier to enforce an observation of all agents in one call to get_action(...), this would not allow the same "level" of asynchronism as offered with MultiAgentEnv, right?

Hmm I was thinking you could allow a subset to be passed in. Similar to how step() in MultiAgentEnv returns a subset of the agents in the env, get_action() could take a subset of the agents as well. So the level of asynchrony would be matching.

That makes sense, thanks for clarification.

As the logic of ExternalMultiAgentEnv would then be analogous to MultiAgentEnv, I'd opt for that.
Currently, this is working as expected: one can pass in a subset of agent observations and get that subset of agent actions back.

python/ray/rllib/env/external_multi_agent_env.py

ericl · 2019-03-03T06:45:49Z

python/ray/rllib/env/external_multi_agent_env.py

+
+
+@PublicAPI
+class ExternalMultiAgentEnv(threading.Thread):


Should this extend ExternalEnv?

The only differences I see between ExternalEnv and ExternalMultiAgentEnv are argument types in the method signatures (e.g. action vs. action_dict), so this should be ok?

python/ray/rllib/env/external_multi_agent_env.py

python/ray/rllib/env/base_env.py

AmplabJenkins · 2019-03-11T07:09:42Z

Can one of the admins verify this patch?

AmplabJenkins · 2019-03-11T21:48:37Z

Can one of the admins verify this patch?

ericl · 2019-03-18T07:19:12Z

@ctombumila37 any update? I can try to help out here unless you've got changes not yet pushed.

append _dict to variables

AmplabJenkins · 2019-03-18T19:16:21Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/12980/
Test PASSed.

AmplabJenkins · 2019-03-19T15:37:24Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/13039/
Test PASSed.

AmplabJenkins · 2019-03-19T20:57:07Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/13045/
Test FAILed.

update docs

…rarchy)

AmplabJenkins · 2019-03-28T11:21:27Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/13322/
Test FAILed.

ericl · 2019-03-28T22:19:50Z

@ctombumila37 I saw the new updates, is this ready to review? Any other issues you've found?

ctombumila37 · 2019-03-29T15:46:05Z

Yes, I hit no other issues.

Should I remove my copy-pasta MultiCartpole-Serving example?

ericl

This is looking pretty good.

Let's remove the examples as noted (instead, a unit test in test_external_env will suffice).
I have some comments on further removing some duplication for the _ExternalEnvEpisode helper class.

python/ray/rllib/env/base_env.py

python/ray/rllib/evaluation/policy_evaluator.py

python/ray/rllib/examples/serving/multiagent/cartpole_client.py

ericl · 2019-03-29T20:08:14Z

python/ray/rllib/env/external_multi_agent_env.py

+            self, action_space, observation_space, max_concurrent)
+
+        # we require to know all agents' spaces
+        if isinstance(self.action_space, dict) or isinstance(self.observation_space, dict):


I noticed sometimes you pass in None for the spaces here -- should that be allowed?

python/ray/rllib/env/external_multi_agent_env.py

AmplabJenkins · 2019-03-29T20:53:25Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/13365/
Test FAILed.

AmplabJenkins · 2019-03-29T21:03:09Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/13367/
Test FAILed.

Remove multiagent cartpole examples.

AmplabJenkins · 2019-04-01T16:40:16Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/13412/
Test FAILed.

Remove _ExternalMultiAgentEnvEpisode

AmplabJenkins · 2019-04-02T09:17:52Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/13435/
Test FAILed.

AmplabJenkins · 2019-04-02T09:39:10Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/13436/
Test FAILed.

AmplabJenkins · 2019-04-02T13:54:25Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/13441/
Test PASSed.

ericl

This looks great! I think we can merge once the unit test is added.

AmplabJenkins · 2019-04-04T23:53:29Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/13552/
Test FAILed.

ericl

LGTM. I pushed some lint fixes

AmplabJenkins · 2019-04-07T02:44:56Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/13608/
Test FAILed.

ericl · 2019-04-07T02:58:20Z

Tests look good, thanks for contributing this!

ctombumila37 and others added 4 commits February 26, 2019 15:41

* start with building towards ExternalMultiAgentEnv

a934614

* current issues:

7731a85

- self.cur_reward in _ExternalEnvEpisode for some reason has to know the ids of all possible agents (serving multiagent example) - the thread of ExternalMultiAgentEnv gets started multiple times, thus ppo.train() deadlocks in the example

Merge branch 'master' into feature/external_multi_agent_env

7ec6caa

* refix multiagent_two_trainers.py example

5dca2c4

ctombumila37 added 2 commits March 1, 2019 09:33

Merge branch 'master' into feature/external_multi_agent_env

65d4c2b

* refactoring:

5c1290e

introduce a flag 'multiagent' in _ExternalEnvToBaseEnv to reduce code duplication.

ctombumila37 changed the title ~~[rllib] ExternalMutliAgentEnv~~ [rllib] [WIP] ExternalMutliAgentEnv Mar 2, 2019

ericl reviewed Mar 3, 2019

View reviewed changes

python/ray/rllib/env/external_multi_agent_env.py Outdated Show resolved Hide resolved

ericl reviewed Mar 3, 2019

View reviewed changes

python/ray/rllib/env/external_multi_agent_env.py Outdated Show resolved Hide resolved

ericl reviewed Mar 9, 2019

View reviewed changes

python/ray/rllib/env/base_env.py Outdated Show resolved Hide resolved

richardliaw changed the title ~~[rllib] [WIP] ExternalMutliAgentEnv~~ [rllib] [WIP] ExternalMultiAgentEnv Mar 10, 2019

ctombumila37 added 2 commits March 18, 2019 16:42

* refactoring:

6c7f218

append _dict to variables

* fix "reward key error" by relying on observation agent keys

99cd4e4

Merge branch 'master' into feature/external_multi_agent_env

669a0d1

ctombumila37 added 3 commits March 27, 2019 11:25

* make ExternalMultiAgentEnv subclass ExternalEnv

e379088

update docs

Merge branch 'master' into feature/external_multi_agent_env

f48b374

* fix precedence of ExternalMultiAgentEnv over ExternalEnv (class hie…

d0431ac

…rarchy)

ctombumila37 force-pushed the feature/external_multi_agent_env branch from 5a704bf to d0431ac Compare March 27, 2019 10:48

- remove redundant policy_server and policy_client (ray-project#4200)

294783f

ctombumila37 marked this pull request as ready for review March 29, 2019 15:46

ericl added 2 commits March 29, 2019 13:07

Update external_multi_agent_env.py

d6d22d1

Update external_multi_agent_env.py

918ed01

ericl requested changes Mar 29, 2019

View reviewed changes

Refactoring (ray-project#4200)

c89dbb3

Remove multiagent cartpole examples.

ctombumila37 added 2 commits April 2, 2019 10:28

Refactoring (ray-project#4200)

774f301

Remove _ExternalMultiAgentEnvEpisode

Merge branch 'master' into feature/external_multi_agent_env

ea1bf37

Add override import (ray-project#4200)

5034266

ericl self-assigned this Apr 3, 2019

ericl reviewed Apr 4, 2019

View reviewed changes

ctombumila37 added 2 commits April 4, 2019 20:44

Merge branch 'master' into feature/external_multi_agent_env

c3f8f19

[rllib] Add first tests for ExternalMultiAgentEnv (ray-project#4200)

08cc22c

lint

378d19c

ericl approved these changes Apr 7, 2019

View reviewed changes

ericl changed the title ~~[rllib] [WIP] ExternalMultiAgentEnv~~ [rllib] ExternalMultiAgentEnv Apr 7, 2019

ericl merged commit 7746d20 into ray-project:master Apr 7, 2019

ctombumila37 deleted the feature/external_multi_agent_env branch April 13, 2019 20:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[rllib] ExternalMultiAgentEnv #4200

[rllib] ExternalMultiAgentEnv #4200

ctombumila37 commented Feb 28, 2019 •

edited by ericl

Loading

AmplabJenkins commented Feb 28, 2019

ericl commented Mar 1, 2019

AmplabJenkins commented Mar 1, 2019

AmplabJenkins commented Mar 1, 2019

ericl Mar 3, 2019

ctombumila37 Mar 20, 2019

ericl Mar 20, 2019

ctombumila37 Mar 27, 2019 •

edited

Loading

ericl Mar 3, 2019

ctombumila37 Mar 27, 2019 •

edited

Loading

AmplabJenkins commented Mar 11, 2019

AmplabJenkins commented Mar 11, 2019

ericl commented Mar 18, 2019

AmplabJenkins commented Mar 18, 2019

AmplabJenkins commented Mar 19, 2019

AmplabJenkins commented Mar 19, 2019

AmplabJenkins commented Mar 28, 2019

ericl commented Mar 28, 2019

ctombumila37 commented Mar 29, 2019

ericl left a comment

ericl Mar 29, 2019

AmplabJenkins commented Mar 29, 2019

AmplabJenkins commented Mar 29, 2019

AmplabJenkins commented Apr 1, 2019

AmplabJenkins commented Apr 2, 2019

AmplabJenkins commented Apr 2, 2019

AmplabJenkins commented Apr 2, 2019

ericl left a comment •

edited

Loading

AmplabJenkins commented Apr 4, 2019

ericl left a comment

AmplabJenkins commented Apr 7, 2019

ericl commented Apr 7, 2019

[rllib] ExternalMultiAgentEnv #4200

[rllib] ExternalMultiAgentEnv #4200

Conversation

ctombumila37 commented Feb 28, 2019 • edited by ericl Loading

What do these changes do?

Related issue number

To Do

AmplabJenkins commented Feb 28, 2019

ericl commented Mar 1, 2019

AmplabJenkins commented Mar 1, 2019

AmplabJenkins commented Mar 1, 2019

ericl Mar 3, 2019

Choose a reason for hiding this comment

ctombumila37 Mar 20, 2019

Choose a reason for hiding this comment

ericl Mar 20, 2019

Choose a reason for hiding this comment

ctombumila37 Mar 27, 2019 • edited Loading

Choose a reason for hiding this comment

ericl Mar 3, 2019

Choose a reason for hiding this comment

ctombumila37 Mar 27, 2019 • edited Loading

Choose a reason for hiding this comment

AmplabJenkins commented Mar 11, 2019

AmplabJenkins commented Mar 11, 2019

ericl commented Mar 18, 2019

AmplabJenkins commented Mar 18, 2019

AmplabJenkins commented Mar 19, 2019

AmplabJenkins commented Mar 19, 2019

AmplabJenkins commented Mar 28, 2019

ericl commented Mar 28, 2019

ctombumila37 commented Mar 29, 2019

ericl left a comment

Choose a reason for hiding this comment

ericl Mar 29, 2019

Choose a reason for hiding this comment

AmplabJenkins commented Mar 29, 2019

AmplabJenkins commented Mar 29, 2019

AmplabJenkins commented Apr 1, 2019

AmplabJenkins commented Apr 2, 2019

AmplabJenkins commented Apr 2, 2019

AmplabJenkins commented Apr 2, 2019

ericl left a comment • edited Loading

Choose a reason for hiding this comment

AmplabJenkins commented Apr 4, 2019

ericl left a comment

Choose a reason for hiding this comment

AmplabJenkins commented Apr 7, 2019

ericl commented Apr 7, 2019

ctombumila37 commented Feb 28, 2019 •

edited by ericl

Loading

ctombumila37 Mar 27, 2019 •

edited

Loading

ctombumila37 Mar 27, 2019 •

edited

Loading

ericl left a comment •

edited

Loading