[BUG] SliceSampler should return unique IDs when sampling multiple times from the same trajectory
@vmoens is already working on this.
Since Nov 20, 2024.
- Dominant language
- Python
- Stars
- 3.6k
- Forks
- 487
- Avg merge
- 23h 50m
- Merged PRs (30d)
- 209
Description
Describe the bug
When using SliceSampler, with strict_length=False, the documentation recommends the use of split_trajectories. However, if two samples from the same episode are placed next to each other, this produces the wrong output because subsequent samples may have the same trajectory_key despite being logically independent.
To Reproduce
import torch
from tensordict import TensorDict
from torchrl.collectors.utils import split_trajectories
from torchrl.data import ReplayBuffer, LazyTensorStorage, SliceSampler
rb = ReplayBuffer(storage=LazyTensorStorage(max_size=1000),
sampler=SliceSampler(
slice_len=5, traj_key="episode",strict_length=False
))
ep_1 = TensorDict(
{"obs": torch.arange(100),
"episode": torch.zeros(100),},
batch_size=[100]
)
ep_2 = TensorDict(
{"obs": torch.arange(4),
"episode": torch.ones(4),},
batch_size=[4]
)
rb.extend(ep_1)
rb.extend(ep_2)
s = rb.sample(50)
t = split_trajectories(s, trajectory_key="episode")
split_trajectories returns nonsense results when trajectory_key contains non-contiguous duplicates.
Even if that weren't the case, there would still be a bug:
When SliceSampler is drawing from relatively few trajectories, there will be situations where multiple slices of the same trajectory are returned next to each other:
episode 0 0 0 0 0 0 0 0 0 0...
obs 2 3 4 5 6 41 42 43 44 45...
|-1st slice-| |-2nd slice--|
However, split_trajectories will see that episode is the same for both slices, and incorrectly combine them into one longer slice.
Expected behavior
SliceSampler should add an additional key to its returned dict to distinguish samples, at least when strict_length=False:
episode 0 0 0 0 0 0 0 0 0 0...
obs 2 3 4 5 6 41 42 43 44 45...
slice 0 0 0 0 0 1 1 1 1 1
Screenshots
If applicable, add screenshots to help explain your problem.
System info
M1 Mac, version 15.1
import torchrl, numpy, sys
print(torchrl.__version__, numpy.__version__, sys.version, sys.platform)
0.6.0+7bf320c 1.26.4 3.11.9 (main, Apr 19 2024, 11:44:45) [Clang 14.0.6 ] darwin
Both torchrl and tensordict were installed from source.
Checklist
- I have checked that there is no similar issue in the repo (required)
- I have read the documentation (required)
- I have provided a minimal working example to reproduce the bug (required)
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Assessment
This issue has not been assessed yet.