[BugFix] RoundRobinWriter, possible duplicated code in the extend method

[albertbou92]

Description

In the class RoundRobinWriter, in the extend method, seems like some of the conditions could be unnecessary. Maybe I am missing something, but if

cur_size = self._cursor

seems like the last two conditions are duplicated code. The code raises no error, but maybe the extra conditions can be removed.

Types of changes

What types of changes does your code introduce? Remove all that do not apply:

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds core functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation (update in the documentation)
• Example (update in the folder of examples)

Checklist

Go over all the following points, and put an x in all the boxes that apply.
If you are unsure about any of these, don't hesitate to ask. We are here to help!

• I have read the CONTRIBUTION guide (required)
• My change requires a change to the documentation.
• I have updated the tests accordingly (required for a bug fix or a new feature).
• I have updated the documentation accordingly.

[codecov]

Codecov Report

Merging #709 (aff12fb) into main (3105819) will increase coverage by 0.04%.
The diff coverage is n/a.

❗ Current head aff12fb differs from pull request most recent head 13b1f23. Consider uploading reports for the commit 13b1f23 to get more accurate results

@@            Coverage Diff             @@
##             main     #709      +/-   ##
==========================================
+ Coverage   88.77%   88.81%   +0.04%     
==========================================
  Files         122      122              
  Lines       21151    21142       -9     
==========================================
+ Hits        18776    18778       +2     
+ Misses       2375     2364      -11     

Flag	Coverage Δ
habitat-gpu	24.36% <ø> (ø)
linux-cpu	84.90% <ø> (+0.03%)	⬆️
linux-gpu	85.77% <ø> (+0.03%)	⬆️
linux-jumanji	29.30% <ø> (ø)
linux-outdeps-gpu	72.10% <ø> (+0.02%)	⬆️
linux-stable-cpu	84.76% <ø> (+0.03%)	⬆️
linux-stable-gpu	85.47% <ø> (+0.04%)	⬆️
linux_examples-gpu	42.74% <ø> (ø)
macos-cpu	84.58% <ø> (+0.03%)	⬆️
olddeps-gpu	74.78% <ø> (+0.02%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
torchrl/data/replay_buffers/writers.py	94.73% <ø> (+16.01%)	⬆️
torchrl/envs/vec_env.py	69.06% <0.00%> (+0.50%)	⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[vmoens]

LGTM! The implementation was odd, I agree.
Could you add a test in TestPrototypeBuffers to check that the cursor moves to the right position in the following cases:

1. Fill the replay buffer with less data than the storage max size (N < M)
2. Fill the replay buffer with as much data as storage max size (N=M), and add some more data in a second time with less data (N<M) => the cursor should be at N
3. Fill the replay buffer directly with more data than the storage max size (N > M), the cursor should be at N-M

[vmoens]

LGTM!
Thanks agains for spotting this and solving it!

[albertbou92]

I think the extend method in the ReplayBuffer from replay_buffers.py also can be simplified. I added the same changes I suggested for RoundRobinWriter and also added tests.

[vmoens]

I think the extend method in the ReplayBuffer from replay_buffers.py also can be simplified. I added the same changes I suggested for RoundRobinWriter and also added tests.

Cool thanks for that too!
Let's wait for the tests to pass and we're good to go