pytorch / rl

A modular, primitive-first, python-first PyTorch library for Reinforcement Learning.
https://pytorch.org/rl
MIT License
2.05k stars 273 forks source link

[BUG] Double Initialization of Priority for New Samples in PrioritizedSampler #2211

Closed wertyuilife2 closed 1 month ago

wertyuilife2 commented 1 month ago

Describe the bug

This issue comes from the original issue #2205.

When ReplayBuffer._add() is called, the following sequence occurs: (1) _writer.add() -> _storage.__setitem__() -> buffer.mark_update() -> _sampler.mark_update() -> _sampler.update_priority() (2) _sampler.add() -> _sampler._add_or_extend()

Both _sampler._add_or_extend() and _sampler.update_priority() update the priority, with update_priority() even applying additional transformations (e.g., torch.pow(priority + self._eps, self._alpha)). This behavior is also present in ReplayBuffer._extend().

This behavior is not reasonable. I believe the mark_update mechanism is somewhat redundant. We do not need to ensure that _attached_entities are updated when changing storage content. Any additional updates required after directly modifying _storage should be the responsibility of the user. mark_update can lead to redundant calls and even cause conflicts.

Additional context

See discussion in the original issue #2205.

Checklist

vmoens commented 1 month ago

Should be solved by #2202