Skip to content

[rllib] Propagate model options correctly in ARS / ES, to action dist of PPO#2974

Merged
ericl merged 10 commits into
ray-project:masterfrom
ericl:options-prop
Oct 1, 2018
Merged

[rllib] Propagate model options correctly in ARS / ES, to action dist of PPO#2974
ericl merged 10 commits into
ray-project:masterfrom
ericl:options-prop

Conversation

@ericl

@ericl ericl commented Sep 27, 2018

Copy link
Copy Markdown
Contributor

@AmplabJenkins

Copy link
Copy Markdown

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

@AmplabJenkins

Copy link
Copy Markdown

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

@ericl ericl changed the title [rllib] Support custom models for ES / ARS [rllib] Propagate model options correctly in ARS / ES, to action dist of PPO Sep 27, 2018
@AmplabJenkins

Copy link
Copy Markdown

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

@AmplabJenkins

Copy link
Copy Markdown

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

@AmplabJenkins

Copy link
Copy Markdown

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

@AmplabJenkins

Copy link
Copy Markdown

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

Comment thread python/ray/rllib/agents/ars/policies.py Outdated
action_noise_std,
options=options)
model_options, action_noise_std):
GenericPolicy.__init__(self, sess, action_space, preprocessor,

@eugenevinitsky eugenevinitsky Sep 28, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like this policy won't be linear? you could make this model.update({"fcnet_hiddens": []}) though that's kind of gross.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm I think that should work. I rewrote it to use the standard model option instead of explicitly having a linear policy.

@AmplabJenkins

Copy link
Copy Markdown

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


>>> dist_cls, dist_dim = ModelCatalog.get_action_dist(env.action_space)
>>> model = ModelCatalog.get_model(inputs, dist_dim)
>>> dist_cls, dist_dim = ModelCatalog.get_action_dist(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why these changes (making config/option mandatory)?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This prevents you from forgetting to specify them. We should have made it mandatory in the first place.

@@ -0,0 +1,131 @@
"""Collection of Carla scenarios, including those from the CoRL 2017 paper."""

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you need future import here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These files are just moved verbatim.

@@ -28,14 +28,14 @@
"carla": {
"run": "PPO",
"env": "carla_env",
"trial_resources": {"cpu": 4, "gpu": 1},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why does Carla no longer need GPU specification?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's auto-detected for rllib since 0.5.0.

@ericl ericl merged commit b45bed4 into ray-project:master Oct 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants