[rllib] Pull out multi-gpu optimizer as a generic class #1313

ericl · 2017-12-12T02:10:54Z

What do these changes do?

First cut at pulling out the multi-gpu optimizer, by adding it for DQN. New classes:

LocalMultiGPUOptimizer
SampleBatch

Pong results with 42x42 and multi_gpu on and off. Perf with multi_gpu is a bit lower, but that's to be expected since we have to do a separate pass for prioritization.

 - DQN_PongDeterministic-v4_0_multi_gpu=True:   TERMINATED [pid=63073], 9824 s, 1270000 ts, 20 rew
 - DQN_PongDeterministic-v4_2_multi_gpu=True:   TERMINATED [pid=63462], 27332 s, 3741000 ts, 20.1 rew
 - DQN_PongDeterministic-v4_4_multi_gpu=True:   TERMINATED [pid=63475], 15352 s, 2016000 ts, 20 rew
 - DQN_PongDeterministic-v4_6_multi_gpu=True:   TERMINATED [pid=63492], 13921 s, 1815000 ts, 20 rew
 - DQN_PongDeterministic-v4_1_multi_gpu=False:  TERMINATED [pid=63211], 7237 s, 1021000 ts, 20 rew
 - DQN_PongDeterministic-v4_3_multi_gpu=False:  TERMINATED [pid=63464], 13237 s, 1909000 ts, 20 rew
 - DQN_PongDeterministic-v4_5_multi_gpu=False:  TERMINATED [pid=63485], 6648 s, 940000 ts, 20.1 rew
 - DQN_PongDeterministic-v4_7_multi_gpu=False:  TERMINATED [pid=63511], 8436 s, 1204000 ts, 20 rew

Related issue number

#1291

AmplabJenkins · 2017-12-14T02:02:36Z

Merged build finished. Test FAILed.

AmplabJenkins · 2017-12-14T02:02: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/2778/
Test FAILed.

AmplabJenkins · 2017-12-14T02:26:29Z

Merged build finished. Test FAILed.

AmplabJenkins · 2017-12-14T02:26:30Z

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

AmplabJenkins · 2017-12-14T02:29:34Z

Merged build finished. Test PASSed.

AmplabJenkins · 2017-12-14T02:29:34Z

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

AmplabJenkins · 2017-12-14T02:37:56Z

Merged build finished. Test PASSed.

AmplabJenkins · 2017-12-14T02:37:57Z

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

AmplabJenkins · 2017-12-15T06:00:07Z

Merged build finished. Test FAILed.

AmplabJenkins · 2017-12-15T06:00: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/2798/
Test FAILed.

richardliaw

Some comments; this introduces some nice abstractions.

richardliaw · 2017-12-15T17:59:29Z

python/ray/rllib/optimizers/sample_batch.py

+    def concat(self, other):
+        """Returns a new SampleBatch with each data column concatenated.
+
+        >>> b1 = SampleBatch({"a": [1, 2]})


Should you label this with Examples:?

richardliaw · 2017-12-15T18:02:05Z

python/ray/rllib/optimizers/sample_batch.py

+    """Wrapper around a dictionary with string keys and array-like values.
+
+    For example, {"obs": [1, 2, 3], "reward": [0, -1, 1]} is a batch of three
+    samples, each with an "obs" and "reward" attribute.


Should Shuffle also be a method here?

Hmm possibly. Right now multi gpu optimizer does a more efficient shuffling where it only permutes indices, which seems just as good.

richardliaw · 2017-12-15T18:32:43Z

python/ray/rllib/dqn/base_evaluator.py

@@ -55,20 +55,23 @@ def update_target(self):
        self.dqn_graph.update_target(self.sess)

    def sample(self):
-        output = []
+        obs, actions, rewards, new_obs, dones = [], [], [], [], []


Does it make sense to replace this with a SyncSampler? A little modification would need to be made to support exploration, but this class will end up cleaner. Can be done in another PR I guess

Sure, I think it should work.

richardliaw · 2017-12-15T18:35:27Z

python/ray/rllib/optimizers/sample_batch.py

+import numpy as np
+
+
+class SampleBatch(object):


So this is a nice abstraction - perhaps it makes sense to extend PartialRollout from this which provides the ability to support truncated rollouts. If so, we can do it in a separate PR.

richardliaw · 2017-12-15T18:47:57Z

python/ray/rllib/dqn/replay_evaluator.py

+            for row in s.rows():
+                self.replay_buffer.add(
+                    row["obs"], row["actions"], row["rewards"], row["new_obs"],
+                    row["dones"])

        if no_replay:
            return samples

        # Then return a batch sampled from the buffer
        if self.config["prioritized_replay"]:


I hope later on all the ReplayBuffer casework can get pushed into the ReplayBuffer

It's difficult. The prioritization requires extra computation that has to be part of the loss.

richardliaw · 2017-12-15T18:49:06Z

python/ray/rllib/dqn/replay_evaluator.py

            self._update_priorities_if_needed()
-            self.samples_to_prioritize = (
-                obses_t, actions, rewards, obses_tp1, dones, batch_idxes)
+            batch = SampleBatch({


you could just have the replay_buffer return a SampleBatch right?

Yes, but I'm less concerned about that for now -- I'd rather keep replay_buffer unchanged as long as possible since it's still basically similar to the baselines code.

richardliaw · 2017-12-15T18:51:13Z

python/ray/rllib/optimizers/multi_gpu.py

+            [ph for _, ph in self.loss_inputs],
+            self.per_device_batch_size,
+            lambda *ph: self.local_evaluator.build_tf_loss(ph),
+            self.config.get("logdir", "/tmp/ray"))


why this directory? didn't logging directory change per #1297

Changed it to os.getcwd()

AmplabJenkins · 2017-12-15T19:39:19Z

Merged build finished. Test FAILed.

AmplabJenkins · 2017-12-15T19:39:20Z

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

AmplabJenkins · 2017-12-15T19:53:59Z

Merged build finished. Test FAILed.

AmplabJenkins · 2017-12-15T19:54:00Z

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

AmplabJenkins · 2017-12-15T22:53:27Z

Merged build finished. Test FAILed.

AmplabJenkins · 2017-12-15T22:53:28Z

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

richardliaw · 2017-12-17T21:29:35Z

Jenkins is failing, but otherwise changes are fine

AmplabJenkins · 2017-12-17T22:24:56Z

Merged build finished. Test PASSed.

AmplabJenkins · 2017-12-17T22:24:57Z

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

ericl added 30 commits November 20, 2017 17:00

interface file

49dd02b

add dqn evaluator

cd50cdd

Mon Nov 20 17:44:44 PST 2017

c5ef83e

implement more

3005f9e

optdoc

95b62c7

add base eval

caed1df

Wed Nov 29 14:27:56 PST 2017

00e1e1c

Merge remote-tracking branch 'upstream/master' into evaluator

d1e6570

update dqn eval

173e845

doc optimizers

edadd65

fix up graph

edb17de

move optimizers

293becc

implement async optimizer

0f17fde

add async mode dqn test

bab5010

update

8b5b1a0

2 workers in test

de44c91

wip need to make build_loss reuse the vars properly

9839db3

wip sample batches

7e1f9e2

wip

b9764c2

cleanup some

48a349c

review comments

916f25b

Merge remote-tracking branch 'upstream/master' into evaluator

85cfe8d

remove logger

191b159

make yamld ep soft

1c4ef0d

fix importer

c8acce0

Merge branch 'evaluator' into evaluator-multi-gpu

2043a04

Merge remote-tracking branch 'upstream/master' into evaluator

1e223e8

Merge branch 'evaluator' into evaluator-multi-gpu

cf2eabd

fix serialization issues with sample batches

e36dafd

multi gpu now working

05fca04

add tuned result

ff4b416

ericl changed the title ~~[rllib] [WIP] Pull out multi-gpu optimizer as a generic class~~ [rllib] Pull out multi-gpu optimizer as a generic class Dec 15, 2017

richardliaw reviewed Dec 15, 2017

View reviewed changes

ericl added 3 commits December 15, 2017 11:18

comments

d35a818

Merge remote-tracking branch 'upstream/master' into evaluator-multi-gpu

5e14b5e

fix a3c import

c9aa526

Merge remote-tracking branch 'upstream/master' into evaluator-multi-gpu

2e5bda1

richardliaw approved these changes Dec 17, 2017

View reviewed changes

fix jenkins

9906c54

richardliaw mentioned this pull request Dec 17, 2017

[rllib] Evaluators and Optimizers Refactoring #1339

Merged

4 tasks

richardliaw merged commit 47b1f02 into ray-project:master Dec 17, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[rllib] Pull out multi-gpu optimizer as a generic class #1313

[rllib] Pull out multi-gpu optimizer as a generic class #1313

ericl commented Dec 12, 2017 •

edited

Loading

AmplabJenkins commented Dec 14, 2017

AmplabJenkins commented Dec 14, 2017

AmplabJenkins commented Dec 14, 2017

AmplabJenkins commented Dec 14, 2017

AmplabJenkins commented Dec 14, 2017

AmplabJenkins commented Dec 14, 2017

AmplabJenkins commented Dec 14, 2017

AmplabJenkins commented Dec 14, 2017

AmplabJenkins commented Dec 15, 2017

AmplabJenkins commented Dec 15, 2017

richardliaw left a comment

richardliaw Dec 15, 2017

ericl Dec 15, 2017

richardliaw Dec 15, 2017

ericl Dec 15, 2017

richardliaw Dec 15, 2017

ericl Dec 15, 2017

richardliaw Dec 15, 2017

richardliaw Dec 15, 2017

ericl Dec 15, 2017

richardliaw Dec 15, 2017

ericl Dec 15, 2017

richardliaw Dec 15, 2017

ericl Dec 15, 2017

AmplabJenkins commented Dec 15, 2017

AmplabJenkins commented Dec 15, 2017

AmplabJenkins commented Dec 15, 2017

AmplabJenkins commented Dec 15, 2017

AmplabJenkins commented Dec 15, 2017

AmplabJenkins commented Dec 15, 2017

richardliaw commented Dec 17, 2017 •

edited

Loading

AmplabJenkins commented Dec 17, 2017

AmplabJenkins commented Dec 17, 2017

[rllib] Pull out multi-gpu optimizer as a generic class #1313

[rllib] Pull out multi-gpu optimizer as a generic class #1313

Conversation

ericl commented Dec 12, 2017 • edited Loading

What do these changes do?

Related issue number

AmplabJenkins commented Dec 14, 2017

AmplabJenkins commented Dec 14, 2017

AmplabJenkins commented Dec 14, 2017

AmplabJenkins commented Dec 14, 2017

AmplabJenkins commented Dec 14, 2017

AmplabJenkins commented Dec 14, 2017

AmplabJenkins commented Dec 14, 2017

AmplabJenkins commented Dec 14, 2017

AmplabJenkins commented Dec 15, 2017

AmplabJenkins commented Dec 15, 2017

richardliaw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AmplabJenkins commented Dec 15, 2017

AmplabJenkins commented Dec 15, 2017

AmplabJenkins commented Dec 15, 2017

AmplabJenkins commented Dec 15, 2017

AmplabJenkins commented Dec 15, 2017

AmplabJenkins commented Dec 15, 2017

richardliaw commented Dec 17, 2017 • edited Loading

AmplabJenkins commented Dec 17, 2017

AmplabJenkins commented Dec 17, 2017

ericl commented Dec 12, 2017 •

edited

Loading

richardliaw commented Dec 17, 2017 •

edited

Loading