-
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 time dimension shaping for PyTorch RNN models. #21735
[RLlib] Fix time dimension shaping for PyTorch RNN models. #21735
Conversation
|
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.
would be nice to create a small unit test for add_time_dimension() based on your research.
good debugging btw.
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.
|
2fb7922
to
991f764
Compare
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.
|
991f764
to
d960ec7
Compare
Hey @XuehaiPan , thanks a ton for this PR! Could you check the failing test cases here? Some things may have been broken by your changes. |
d960ec7
to
3db5595
Compare
Signed-off-by: Xuehai Pan <[email protected]>
Signed-off-by: Xuehai Pan <[email protected]>
Signed-off-by: Xuehai Pan <[email protected]>
Signed-off-by: Xuehai Pan <[email protected]>
Signed-off-by: Xuehai Pan <[email protected]>
Signed-off-by: Xuehai Pan <[email protected]>
Signed-off-by: Xuehai Pan <[email protected]>
2c3484d
to
76bf9e6
Compare
Signed-off-by: Xuehai Pan <[email protected]>
76bf9e6
to
426ff69
Compare
Hi @sven1977, I updated my PR. The CI test errors seem irrelevant. |
Merged! :) Thanks for the fix @XuehaiPan. |
Why are these changes needed?
tensor.reshape
totensor.transpose
when storing data in time-major format. See [RLlib] Bug when using RNNs, time_major and PyTorch #13075 (comment) for more details.tensor.view
instead oftensor.reshape
, which will not create a new tensor instance.Related issue number
Closes #13075
Checks
scripts/format.sh
to lint the changes in this PR.