Open MihaiAnca13 opened 3 years ago
Hi! thanks for your contribution!, great first issue!
@MihaiAnca13 good point, mind sending a PR?
@Borda thanks for your reply. Can you please have a quick look at the SharedReplayBuffer I've added? I'm not sure what I should do/add next to make this useful.
🚀 Feature
The RL implementations added do not have the num_workers option. I have a feeling this is because the code doesn't support a shared replay buffer.
Motivation
Adding this would enable distributed training, which is very important in RL. Certain models wouldn't work at all without this (e.g.: A3C).
Pitch
When num_workers args is specified and is greater than 1, the replay buffer used should be shared amongst all workers.
Alternatives
I believe OpenAI does this using mpi4py, but it would probably defeat the point since PyTorch handles multiprocessing.
Additional context
In the PyTorch docs, they said: "When num_workers > 0, each worker process will have a different copy of the dataset object". This is not true. I tested it using
torch.utils.data.get_worker_info()
and each worker was pointing to the same address. This is good because it means the replay buffer can be initialised in the Iterable class.The code below is what I've used for testing. Line 27 (
self.replay_buffer.share_memory_()
) is where the magic happens. Try running it with that line commented and uncommented to see the difference. I'm happy to provide more explanation if needed or to help implement this.