-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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 default view_requirement in policy.py #27255
Conversation
This reverts commit 74686a8.
…new_obs key, for inference we should not look into the future. Signed-off-by: Kourosh Hakhamaneshi <[email protected]>
Signed-off-by: Kourosh Hakhamaneshi <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good. should we mark INFOS column not_for_compute_action as well?
last time I had trouble with this, I made some custom changes in RandomPolicy:
https://github.com/ray-project/ray/blob/master/rllib/examples/policy/random_policy.py#L36-L40
if we flip the default values here, then we can get rid of the custom fix.
I don't think info dict will ever be able to get batched and used for forward pass directly?
None of the other default values should hit this particular issue, since they are not forward looking views? |
right, they should not hit the particular issue. |
Signed-off-by: Kourosh Hakhamaneshi <[email protected]>
Signed-off-by: Stefan van der Kleij <[email protected]>
Why are these changes needed?
the default view_requirement in policy.py used to enable used_for_compute_actions for new_obs key; for inference we should not look into the future.
Related issue number
Checks
git commit -s
) in this PR.scripts/format.sh
to lint the changes in this PR.