-
Notifications
You must be signed in to change notification settings - Fork 333
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
[Feature Request] Replay Buffer features #994
Comments
if it's with replacement you never are out of data are you? If it's without (
For your final point i"m open to discuss it as long as we can still iterate over the buffer as we do now, which requires the batch_size to be set in advance. EDIT: I was peeking at reddit this weekend and the main criticism to torchrl is that it is too complex. I feel that passing the batch-size every time is more complex than not. It's a special use case: in most cases the batch size does not change and I think we should not constrain users to pass it every time. Happy to support the feature you're asking for but not sure it should be the default. |
And what will I sample in the case I am out of data, the last valid data before I ran out? Don't you find it strange to query an empty buffer and get data in your normal batch_size?
I see the point about prefetch. But for drop_last, couldn't the sampler just look at how much data is in the buffer and decide based on that on the spot? With a buffer in my idea we have some stored data. Now, at each time I should be able to take as much as I want and decide if to use replacement or not on the spot. (unless i prefetech things, which will of course not allow me to change my mind) |
There are two different things: The other is
No bc if I'm sampling 128 elements now, and next time I'm sampling 1, and there are 64 left, is the next batch the last or not? It isn't, but there is no way for the sampler to tell.
note that you can share the storage among multiple buffers at no cost, not sure if that would cover what you have in mind. But Final point: the goal of having a composable replay buffer is that if you have an unusual use case in mind you can code your own sampler / storage/ writer. We don't claim we support every single use case, but we certainly claim we want to enable you to do whatever you want without having to re write the whole buffer class! |
I see, thanks for all that info it clears up things. The thing that confused me is buffer = ReplayBuffer(
storage=LazyTensorStorage(frames_per_batch),
sampler=SamplerWithoutReplacement(),
batch_size=frames_per_batch
)
buffer.extend(my_data_of_size_frames_per_batch)
buffer.sample() # I get my data
buffer.sample() # doesn't throw an error and what do I get?
The second bag I got had already been sampled and I did not reset anything |
nope bc it resets automatically, unlike the iterator. In practice we could raise an exception when the buffer is empty, happy to see that feature. |
yep it is just that automatic reset which I didn't know about. Maybe the sampler without replacement could have a parameter called auto_reset? |
Solved by #1003 |
Hello,
I was playing around with the replay buffers and found some behaviors which resulted surprising for me and was wondering what is the reasoning for this.
buffer.sample(batch_size)
it says it is deprected and the correct way is to set the batch_size upon init. But what if I want to change the size of my batch sample at every call?buffer.sample()
forever, without any error being thrown. How do I know when I'm out of data? what data am I getting when I am out of data?These are the main two issues I faced in my code.
More generally I would like not to commit at init time to a specific batch_size or replacement stratedy.
I would like at every call to
sample()
to decide how many sample to receive and if I want to use replacement. and an error to be thrown or a sample of size 0 to be given in case I am out of data.The text was updated successfully, but these errors were encountered: