Keep the setting of user created DataLoader in replacing DistributedSampler

[wlkz]

🚀 Feature

Motivation

As mention at #2789, the default behavior of replace_sampler_ddp is creating a new DistributedSampler. The shuffle setting depends on the kind of dataloader (train or val/test dataloader). However, this behavior override the setting of user defined dataloader, such as shuffle or drop_last. A more reasonable solution is to get this setting direly from user created dataloader, and apply the same setting in DistributedSampler.

Pitch

For example, we can get the shuffle setting form dataloader.sampler. If this is a instance of SequentialSampler, shuffle=False.

Alternatives

Set replace_sampler_ddp=False, and handle it by hand.

Additional context

[github-actions]

Hi! thanks for your contribution!, great first issue!

[rohitgr7]

Set replace_sampler_ddp=False, and handle it by hand.

I think this is the ideal way to handle this. Else we will make things more complicated.

[wlkz]

But more I concern is the side effect of the default setting replace_sampler_ddp=True. Both Getting started-Lightning in 2 steps and Common Use Cases-Multi-GPU training do not describe the magic of sampler replacement. In most case it seems correct. But unexpected setting change sometimes make thing worse. For example, some loss function is batch size sensitive, and the flag drop_last=True must be set in dataloader. However, the replaced DistributedSampler ignore this setting, which will give a unexpected gradient update in last batch. In this situation, the error is hard to debug, as the training work well in single GPU, but fail in DDP training. Thing won't get better until you have a look to lengthy API reference of Trainer, and find correct samplers mess up everything.

Set replace_sampler_ddp=False, and handle it by hand.

I think this is the ideal way to handle this. Else we will make things more complicated.

I totally agree with you. If this is a user responsibility to handle it, I suggest to make a brief description about replace_sampler_ddp in Common Use Cases-Multi-GPU training, telling the user the setting drop_last and shuffle would be reset, and handle it by hand in this situation.

[stale]

This issue has been automatically marked as stale because it hasn't had any recent activity. This issue will be closed in 7 days if no further activity occurs. Thank you for your contributions, Pytorch Lightning Team!