-
Notifications
You must be signed in to change notification settings - Fork 624
Expose common dataloader args #2097
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -53,7 +53,14 @@ class ParallelAwareDataloader(StatefulDataLoader, BaseDataLoader): | |
| dp_rank: Data parallelism rank for this dataloader. | ||
| dp_world_size: The world size of the data parallelism. | ||
| batch_size: The batch size to use for each iteration. | ||
| collate_fn: Optional function to collate samples in a batch. | ||
| collate_fn (Callable, optional): A function that takes a list of samples from the | ||
| dataset and collates them into a batch. Defaults to ``None``. | ||
| num_workers: Number of worker processes for data loading. Defaults to 0. | ||
| persistent_workers: If True, keep workers alive between dataset iterations. | ||
| Only applicable when num_workers > 0. Defaults to False. | ||
| prefetch_factor: Number of batches to prefetch per worker. Only applicable | ||
| when num_workers > 0. Defaults to None (uses PyTorch default of 2). | ||
| pin_memory: If True, copy tensors to CUDA pinned memory. Defaults to False. | ||
| """ | ||
|
|
||
| dp_rank: int | ||
|
|
@@ -67,11 +74,23 @@ def __init__( | |
| dp_world_size: int, | ||
| batch_size: int, | ||
| collate_fn: Callable | None = None, | ||
| num_workers: int = 0, | ||
| persistent_workers: bool = False, | ||
| prefetch_factor: int | None = None, | ||
| pin_memory: bool = False, | ||
| ): | ||
| self.dp_world_size = dp_world_size | ||
| self.dp_rank = dp_rank | ||
| self.batch_size = batch_size | ||
| super().__init__(dataset, batch_size, collate_fn=collate_fn) | ||
| super().__init__( | ||
| dataset, | ||
| batch_size, | ||
| collate_fn=collate_fn, | ||
| num_workers=num_workers, | ||
| persistent_workers=persistent_workers, | ||
| prefetch_factor=prefetch_factor, | ||
| pin_memory=pin_memory, | ||
|
Comment on lines
+89
to
+92
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
These are valid concerns. For now I'm leaning towards keeping things simple by passing **kwargs around. Does it make sense if we only make these args explicit when sending to the actual init of |
||
| ) | ||
| self._rank_id = f"dp_rank_{dp_rank}" | ||
|
|
||
| def state_dict(self) -> dict[str, Any]: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you help change this: let's keep at most one positional arg (
dataset) and others to be kwargs.