🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/rl/1989

• 📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

❌ 1 New Failure, 2 Unrelated Failures

As of commit 04dce54 with merge base 87f3437 ():

This comment was automatically generated by Dr. CI and updates every 15 minutes.

$\color{#D29922}\textsf{\Large⚠\kern{0.2cm}\normalsize Warning}$ Result of CPU Benchmark Tests

Total Benchmarks: 91. Improved: $\large\color{#35bf28}10$. Worsened: $\large\color{#d91a1a}7$.

$\color{#D29922}\textsf{\Large⚠\kern{0.2cm}\normalsize Warning}$ Result of GPU Benchmark Tests

Total Benchmarks: 94. Improved: $\large\color{#35bf28}4$. Worsened: $\large\color{#d91a1a}6$.

When I last did this, I didn't get the transforms right the first time (or second, or third...), and it took some iterations - sometimes jointly with algorithm development. Hence, I'd prefer an option not to replace the downloaded dataset, to speed up that iterative process.

Makes sense!
We could:

• Provide a dest keyword arg for the dest. Either it defaults to the source and replaces it -- or we only do it if a "same" dest is provided.
• Provide the option to do only X frames (e.g. max_frames). By default, the preproces will do the entire dataset, but if you can ask for less. This would be mainly useful for debugging

Upon reflection it seems it makes more sense to encourage users to copy the data somewhere else.

What should happen to:

• Sampler? I guess we can generally keep the same sampler as the one set in the dataset
• Writer? I would keep the immutable writer by default
• transforms? Here I would vote for discarding any transform that has been set in the dataset. Transforms are presumably used to do the same job as this preproc

@nicklashansen @dennismalmgren I need some input here.

The issue I'm facing is that there is no way to tell what the storage will look like after the transform, and hence the sampler / writer / transforms will in many cases be obsolete.

If we recycle them by default there's a big chunk of cases where the transformed dataset will not be usable anymore.

Therefore, since we need at least the space for the dataset and the transformed dataset to use this functionality (you can't delete the first before you get the second), I think
(1) we should never replace the dataset with its transformed version: it's a bit chaotic, hard to implement because the dataset can come with many other attributes and on top of that we can't be sure that the signature of the dataset will work with the new one
(2) the transform should not return a replay buffer but just a storage and the user should build the replay buffer from scratch:

dataset = OpenXExperienceReplay(...)
new_storage = dataset.preprocess(func, ...)
# we give users the option to delete the dataset
dataset.clear()
new_dataset = ReplayBuffer(storage=new_storage, sampler=whatever, ...)

A bit more code is needed but at least people know what they're doing. Note that new_dataset = ReplayBuffer(storage=new_storage) will always work with a default writer and sampler.

Wdyt?

@vmoens thank you for initiating this! i agree with @dennismalmgren comment, and in general i see this as being used mostly as a separate step as opposed to being part of the training process / data loading. and i agree that it the most convenient solution would be to store the transformed dataset somewhere else by default, but perhaps giving users the option of deleting the original data after preprocessing. im not overly concerned about the (potentially) 2x disk space requirements.

ideally one would be able to download all of the torchrl datasets to disk, preprocess and save them in the desired format (would probably have to be specified manually but so be it), and then whichever training process and data loader that users build downstream (most likely in a different process) will use that new, common (in case of multi-dataset training) format without the added overhead of preprocessing on the fly. that said, i can imagine scenarios in which a user might still want to apply some transforms when sampling, e.g., data augmentation or random slicing, but that seems like something that would be supported as is?

Yes since you're rebuilding the replay buffer you can pass whichever transform.

I made the changes you can check them here
https://docs-preview.pytorch.org/pytorch/rl/1989/reference/generated/torchrl.data.datasets.BaseDatasetExperienceReplay.html#torchrl.data.datasets.BaseDatasetExperienceReplay.preprocess

Thanks! LGTM at first glance, I'm a bit caught up in other things atm but will give it a try soon!