-
-
Notifications
You must be signed in to change notification settings - Fork 390
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
DDPLoaderWrapper update #1385
DDPLoaderWrapper update #1385
Conversation
catalyst/data/ddp_loader.py
Outdated
# https://github.com/huggingface/accelerate/blob/main/src/accelerate/data_loader.py | ||
class BatchSamplerShard(BatchSampler): | ||
""" | ||
Wraps a PyTorch :obj:`BatchSampler` to generate batches for one of the processes only. Instances of this class will |
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.
[pep8] reported by reviewdog 🐶
E501 line too long (119 > 99 characters)
catalyst/data/ddp_loader.py
Outdated
# https://github.com/huggingface/accelerate/blob/main/src/accelerate/data_loader.py | ||
class BatchSamplerShard(BatchSampler): | ||
""" | ||
Wraps a PyTorch :obj:`BatchSampler` to generate batches for one of the processes only. Instances of this class will |
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.
[pep8] reported by reviewdog 🐶
W505 doc line too long (119 > 99 characters)
catalyst/data/ddp_loader.py
Outdated
class BatchSamplerShard(BatchSampler): | ||
""" | ||
Wraps a PyTorch :obj:`BatchSampler` to generate batches for one of the processes only. Instances of this class will | ||
always yield a number of batches that is a round multiple of :obj:`num_processes` and that all have the same size. |
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.
[pep8] reported by reviewdog 🐶
E501 line too long (118 > 99 characters)
catalyst/data/ddp_loader.py
Outdated
class BatchSamplerShard(BatchSampler): | ||
""" | ||
Wraps a PyTorch :obj:`BatchSampler` to generate batches for one of the processes only. Instances of this class will | ||
always yield a number of batches that is a round multiple of :obj:`num_processes` and that all have the same size. |
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.
[pep8] reported by reviewdog 🐶
W505 doc line too long (118 > 99 characters)
catalyst/data/ddp_loader.py
Outdated
""" | ||
Wraps a PyTorch :obj:`BatchSampler` to generate batches for one of the processes only. Instances of this class will | ||
always yield a number of batches that is a round multiple of :obj:`num_processes` and that all have the same size. | ||
Depending on the value of the :obj:`drop_last` attribute of the batch sampler passed, it will either stop the |
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.
[pep8] reported by reviewdog 🐶
E501 line too long (113 > 99 characters)
length = len(self.batch_sampler) // self.num_processes | ||
return length if self.drop_last else length + 1 | ||
|
||
def __iter__(self): |
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.
[pep8] reported by reviewdog 🐶
D105 Missing docstring in magic method
catalyst/data/ddp_loader.py
Outdated
# We gather the initial indices in case we need to circle back at the end. | ||
if not self.drop_last and idx < self.num_processes: | ||
initial_data += batch | ||
# We identify the batch to yield but wait until we ar sure every process gets a full batch before actually |
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.
[pep8] reported by reviewdog 🐶
E501 line too long (118 > 99 characters)
catalyst/data/ddp_loader.py
Outdated
# We gather the initial indices in case we need to circle back at the end. | ||
if not self.drop_last and idx < self.num_processes: | ||
initial_data += batch | ||
# We identify the batch to yield but wait until we ar sure every process gets a full batch before actually |
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.
[pep8] reported by reviewdog 🐶
W505 doc line too long (118 > 99 characters)
catalyst/data/ddp_loader.py
Outdated
while len(initial_data) < self.num_processes * self.batch_size: | ||
initial_data += initial_data | ||
|
||
# If the last batch seen was of the proper size, it has been yielded by its process so we move to the next |
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.
[pep8] reported by reviewdog 🐶
E501 line too long (118 > 99 characters)
catalyst/data/ddp_loader.py
Outdated
while len(initial_data) < self.num_processes * self.batch_size: | ||
initial_data += initial_data | ||
|
||
# If the last batch seen was of the proper size, it has been yielded by its process so we move to the next |
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.
[pep8] reported by reviewdog 🐶
W505 doc line too long (118 > 99 characters)
catalyst/data/ddp_loader.py
Outdated
if not self.drop_last and idx < self.num_processes: | ||
initial_data += batch | ||
# We identify the batch to yield | ||
# but wait until we are sure every process gets a full batch |
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.
[pep8] reported by reviewdog 🐶
W291 trailing whitespace
Pull Request FAQ
Description
Related Issue
Type of Change
PR review
Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.
Checklist
latest
andminimal
requirements?