-
Notifications
You must be signed in to change notification settings - Fork 880
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
Allow for ACCELERATE_SEED env var #2126
Conversation
The documentation is not available anymore as the PR was closed or merged. |
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.
Implementation-wise, this looks good to me. I think the documentation could be improved, LMK if you agree.
src/accelerate/utils/random.py
Outdated
@@ -30,7 +31,8 @@ | |||
|
|||
def set_seed(seed: int, device_specific: bool = False): | |||
""" | |||
Helper function for reproducible behavior to set the seed in `random`, `numpy`, `torch`. | |||
Helper function for reproducible behavior to set the seed in `random`, `numpy`, `torch`. Also will set the random | |||
seed for `SeedableRandomSampler` through the `ACCELERATE_SEED` environment variable. |
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.
Similar comment to the one above: Users don't really care about SeedableRandomSampler
, as this is an implementation detail of accelerate. They will care more about knowing when this function needs to be used.
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.
Nice and simple. So in the end, users should call set_seed
to ensure reproducibility and that's enough, right?
@BenjaminBossan yep! set_seed, and in the Trainer just having it set the torch seed will be enough for general users |
What does this PR do?
This PR introduces
ACCELERATE_SEED
as part ofset_seed
. Doing so will allow for the custom random samplers to rely on it for setting the seed, rather than the generator (as this can lead to imperfect issues). Solves related trainer test failures.Fixes # (issue)
Internal slack thread
Before submitting
Pull Request section?
to it if that's the case.
documentation guidelines, and
here are tips on formatting docstrings.
Who can review?
Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.
@LysandreJik @BenjaminBossan