-
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] Issue 22625: MultiAgentBatch.timeslices()
does not behave as expected.
#22657
[RLlib] Issue 22625: MultiAgentBatch.timeslices()
does not behave as expected.
#22657
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.
this is a nice fix.
Can we add a small test to prove that this does the fixes that you're attempting to make? |
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.
lgtm!
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.
1 nit.
my favorite kind of PR. little code change, tons of tests :)
MultiAgentBatch.timeslices()
does not behave as expected.
Why are these changes needed?
MultiAgentBatch.timeslices() does not behave as expected. I returns non-information-batches for later slices and does not do the same for earlier slices.
timeit tells me that
cur_slice.clear()
is quicker thancur_slice = collections.defaultdict(SampleBatchBuilder)
.Related issue number
Closes #22625
Checks
scripts/format.sh
to lint the changes in this PR.