Skip to content
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

More flexible RNG synchronization #8

Merged
merged 2 commits into from
Mar 9, 2021
Merged

More flexible RNG synchronization #8

merged 2 commits into from
Mar 9, 2021

Conversation

sgugger
Copy link
Collaborator

@sgugger sgugger commented Mar 9, 2021

Currently, the RNG synchronization at the beginning of each epoch is done very broadly, which means the potential data augmentation is done the same way on each process. While this is probably not hurting a big deal unless the underlying dataset isn't very big, there is a way to use local RNG with torch.Generator introduced in PyTorch 1.6.

This PR enables this by default and leaves some flexibility to the user to set the RNGs they want to synchronize (among torch, cuda, xla and local generator).

@sgugger sgugger requested a review from LysandreJik March 9, 2021 18:49
Copy link
Member

@LysandreJik LysandreJik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for the in-person explanation :)

@@ -42,6 +44,18 @@ class Accelerator:
cpu (:obj:`bool`, `optional`):
Whether or not to force the script to execute on CPU. Will ignore GPU available if set to :obj:`True` and
force the execution on one process only.
rng_types (list of :obj:`str` or :class:`~accelerate.utils.RNGType`):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this!

@@ -35,28 +44,47 @@ def set_seed(seed: int):
np.random.seed(seed)
torch.manual_seed(seed)
torch.cuda.manual_seed_all(seed)
if is_tpu_available():
xm.set_rng_state(seed)
# ^^ safe to call this function even if cuda is not available
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you can move this comment above if you want to keep it

@sgugger sgugger merged commit e67aa2e into main Mar 9, 2021
@sgugger sgugger deleted the rng_support branch March 9, 2021 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants