[bug] dgl.dataloading.DataLoader forwards args to torch.utils.data.DataLoader but many are not valid
- Dominant language
- Python
- Stars
- 14.3k
- Forks
- 3.1k
- PR merge metrics
- No merged PRs in 30d
Description
## 🐛 Bug
Currently, the DGL's DataLoader is a child of PyTorch's DataLoader, and passes `kwargs` to it, however DGL's dataloader does several things under the hood which breaks many of these options, and no indication is given to the user what is valid and what is not. Further more, CI covers a fraction of possible options users my pass in.
Current problematic options:
* `sampler` and `batch_sampler`: Specifying either of these results in errors about `IterableDataset`. Our documentation explicitly references `sampler` inside the description for the `use_ddp` argument.
* `collate_fn`: This results in a duplicate keyword error.
* `pin_memory`: This results in a duplicate keyword error, as the super is called with `pin_memory=self.pin_prefetcher`.
* `pin_memory_device` (new in 1.12): This will only have effect on the input to the _PrefetchingIterator, which may not affect the data as returned to the user.
* `generator`: Has no effect.
* `timeout`: While this will work relatively correctly with respect to the workers, when a prefetching thread is active, it is possible to block on the dataloader for longer than the specified timeout, which would be unexpected from the user's perspective.
* ~~`worker_init_fn`: This results in a duplicate keyword error, however this appears just to be a bug as we attempt to correctly wrap the passed in function, but fail to delete it from the `kwargs`.~~
* `batch_size=None`: While it would be odd for a user to do this given we don't provide users a way to manually batch data, our documentation does address it when it is valid for PyTorch's DataLoader.
We also have the issue of when PyTorch's DataLoader is updated, how to maintain compatibility. For example, the option `pin_memory_device` was just added in 1.12.
## Proposed fix
I think the best way to fix this, is to remove the `kwargs` parameter to our DataLoader, and instead capture every argument explicitly. This way we can limit, verify, and test the different argument combinations passed in. It will also make reading our documentation easier.
## Alternative fix
We could try to keep documentation on what parameters are valid, but we may need separate documentation per PyTorch version.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start at dgl.dataloading.DataLoader and compare its accepted arguments with torch.utils.data.DataLoader. Trace how kwargs are forwarded, including sampler, batch_sampler, collate_fn, pin_memory, generator, timeout, and batch_size. Done means the supported argument set is explicit, incompatible options are handled consistently, and the combinations are covered by tests and documentation.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- data, machine-learning
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100