Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[RLlib] Fix custom multi action distr #13681

Merged
merged 13 commits into from
Jan 28, 2021
Merged

[RLlib] Fix custom multi action distr #13681

merged 13 commits into from
Jan 28, 2021

Conversation

sven1977
Copy link
Contributor

@sven1977 sven1977 commented Jan 25, 2021 •
edited
Loading

Fix custom multi action distr.

Re-vival of this closed PR by zseymour: #11868

When config["custom_action_dist"] is a subclass of MultiActionDistribution or TorchMultiActionDistribution, dst_class became a tuple of (partial, int), resulting in

AttributeError: 'tuple' object has no attribute 'required_model_output_shape'
This pull request instead causes ModelCatalog._get_multi_action_distribution to always return a tuple and returns directly from L148, bypassing the later return that threw the error.

Why are these changes needed?

Related issue number

Checks

  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

zseymour and others added 11 commits November 6, 2020 18:31
@zseymour
Make _get_multi_action_distribution return tuple
db0ffe2 
Fixes custom MultiActionDistributions.
@zseymour
dist_cls -> dist_class
eb4816d
@zseymour
Make _get_multi_action_distribution return tuple
a3a73b2 
Fixes custom MultiActionDistributions.
@zseymour
dist_cls -> dist_class
98831af
@zseymour
Pass full config to _get_multi_action_distribution
265f69a
@zseymour
Add test for custom multi-action distribution
9c7288d
@zseymour
Merge branch 'fix_custom_multi_action_distr' of https://github.com/zs…
6c7440e 
…eymour/ray into fix_custom_multi_action_distr
@zseymour
Format
0e3b616
@sven1977
Merge branch 'master' of https://github.com/ray-project/ray into fix_…
287f940 
…custom_multi_action_distr

# Conflicts:
#	rllib/models/catalog.py
#	rllib/tests/test_catalog.py
@sven1977
wip.
5970575
@sven1977
Merge branch 'master' of https://github.com/ray-project/ray into fix_…
509b0f6 
…custom_multi_action_distr
@sven1977 sven1977 merged commit 4bc257f into ray-project:master Jan 28, 2021
fishbone pushed a commit to fishbone/ray that referenced this pull request Feb 16, 2021
@sven1977 @fishbone
[RLlib] Fix custom multi action distr (ray-project#13681)
44f52af
fishbone added a commit to fishbone/ray that referenced this pull request Feb 16, 2021
@fishbone
Revert "[RLlib] Fix custom multi action distr (ray-project#13681)"
3bbd0f4 
This reverts commit 44f52af.
fishbone added a commit to fishbone/ray that referenced this pull request Feb 16, 2021
@fishbone
Revert "[RLlib] Fix custom multi action distr (ray-project#13681)"
6b2c34b 
This reverts commit 44f52af.
@sven1977 sven1977 deleted the fix_custom_multi_action_distr branch June 2, 2023 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@michaelzhiluo michaelzhiluo michaelzhiluo approved these changes

Assignees

@michaelzhiluo michaelzhiluo

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@sven1977 @michaelzhiluo @zseymour