Initialising modules with different weights with DDP is dangerously buried in the doc notes

[epignatelli]

🐛 Bug

More a discussion than a real bug.

The docs for the DDP say:

Make sure to set the random seed before the instantiation of a Trainer() so that each model initializes with the same weights.

In the most common settings, it is considered bad practice to initialise the modules with different parameters according to the GPU.
To me - and this is a personal opinion - it does not make sense in theory as well.

The reason why I posted it as a bug and not as a doc issue is that this is really dangerous, if you don't know about it.
Using a note in the docs does not help either. I don't assume a user reads the entire docs before using a library.

[awaelchli]

So a solution would be to broadcast the weights from process 0 to all others before starting training. Is this correct? Then setting the seed is optional.

[epignatelli]

I am not sure what the solution might be, as my knowledge about lightning is poor.
On a high level, I guess passing the seed would be the minimal information pytorch needs to coordinate the initialisations.

[awaelchli]

On a high level, I guess passing the seed would be the minimal information pytorch needs to coordinate the initialisations.

We can't know the seed unless the user sets it, right?
So I believe my suggestion is the only way if we want to allow for random initial conditions while having the same state across processes.

[awaelchli]

@justusschock @tchaton @SeanNaren any thoughts/concerns here?

[justusschock]

@awaelchli You're right, but luckily there is nothing left to do for us besides removing this line from the docs. Since your refactor bases it back to plain torch.nn.DistributedDataParallel, it does that automatically (see here , here and
here)

[awaelchli]

oh you're right, that's awesome. And it only makes sense now that pytorch DDP would take care of this. I sent a PR to remove the outdated info