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.

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

[RLlib] Fix Unity3D built-in examples action bounds from -inf/inf to -1.0/1.0. #22247

Merged
merged 1 commit into from
Feb 10, 2022

Conversation

sven1977
Copy link
Contributor

@sven1977 sven1977 commented Feb 9, 2022

Fix Unity3D built-in examples action bounds from -inf/inf to -1.0/1.0.

If the action space is -inf/inf, unsquashing (e.g. from PPO's normalized action range of roughly -1.0/1.0) will not happen and the env may complain. Unity3D envs usually use action bounds of -1.0/1.0 as indicated by the hard-coded clipping code in Unity's ML-Agents, doing something like:

action = clip(action, -3.0, 3.0) / 3.0 -> always between -1.0 and 1.0.

Why are these changes needed?

#21109

Related issue number

Issue #21109

Closes #21109

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 :(

@sven1977 sven1977 added the tests-ok The tagger certifies test failures are unrelated and assumes personal liability. label Feb 9, 2022
Copy link
Member

@avnishn avnishn left a comment

Choose a reason for hiding this comment

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

Does unity automatically do space scaling? Seems fine if they do, just so I'm understanding this correctly. I guess another way of asking my question is: what are the bounds of the underlying space on the unity side? Do we define them however we want to, or does unity?

@sven1977
Copy link
Contributor Author

Unity (the env) does.

@sven1977 sven1977 merged commit 1c791b7 into ray-project:master Feb 10, 2022
simonsays1980 pushed a commit to simonsays1980/ray that referenced this pull request Feb 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests-ok The tagger certifies test failures are unrelated and assumes personal liability.
Projects
None yet
2 participants