Seed _get_train_sampler's generator with arg seed to improve reproducibility
Author: dlwh
Pull request: yes

... and make the world_size<=1 code path more similar to the others

# What does this PR do?

Seed the generator that is (sometimes) used in _get_train_sampler with self.args.seed to improve reproducibility/control over randomness.

https://github.com/huggingface/transformers/pull/11582/files introduced using torch generators (when possible) to improve reproducibility in trainer's _get_train_sampler. However, the seed in TrainingArguments isn't used explicitly. This is usually fine, but if there's any non-determinism in the number of calls to random generators in, e.g., model_init, this doesn't do what you want. (For instance, let's say you wanted to play with differently sized models but leave all else equal including data loading order.) 

The other branches of this method appear (I think) to all use this same seed, so that helps bring this in line too, but that's probably not important.

## Before submitting
- [ ] This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
- [x] Did you read the [contributor guideline](https://github.com/huggingface/transformers/blob/master/CONTRIBUTING.md#start-contributing-pull-requests),
      Pull Request section?
- [ ] Was this discussed/approved via a Github issue or the [forum](https://discuss.huggingface.co/)? Please add a link
      to it if that's the case.
No, hope that's ok.
- [ ] Did you make sure to update the documentation with your changes? Here are the
      [documentation guidelines](https://github.com/huggingface/transformers/tree/master/docs), and
      [here are tips on formatting docstrings](https://github.com/huggingface/transformers/tree/master/docs#writing-source-documentation).
internal bugfix

- [x] Did you write any new necessary tests?


## 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.

Suggesting @sgugger (cc @siddk) 

<!-- Your PR will be replied to more quickly if you can figure out the right person to tag with @

 If you know how to use git blame, that is the easiest way, otherwise, here is a rough guide of **who to tag**.
 Please tag fewer than 3 people.

Models:

- albert, bert, xlm: @LysandreJik
- blenderbot, bart, marian, pegasus, encoderdecoder,  t5: @patrickvonplaten, @patil-suraj
- longformer, reformer, transfoxl, xlnet: @patrickvonplaten
- fsmt: @stas00
- funnel: @sgugger
- gpt2: @patrickvonplaten, @LysandreJik
- rag: @patrickvonplaten, @lhoestq
- tensorflow: @LysandreJik

Library:

- benchmarks: @patrickvonplaten
- deepspeed: @stas00
- ray/raytune: @richardliaw, @amogkam
- text generation: @patrickvonplaten
- tokenizers: @n1t0, @LysandreJik
- trainer: @sgugger
- pipelines: @LysandreJik

Documentation: @sgugger

HF projects:

- datasets: [different repo](https://github.com/huggingface/datasets)
- rust tokenizers: [different repo](https://github.com/huggingface/tokenizers)

Examples:

- maintained examples (not research project or legacy): @sgugger, @patil-suraj
- research_projects/bert-loses-patience: @JetRunner
- research_projects/distillation: @VictorSanh

 -->


Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_15961). All of your documentation changes will be reflected on that endpoint.

No. As mentioned in the other issues, this code is intended to be this way. The torch RNG has been seeded so this is completely deterministic.

Ok, I think it depends on what guarantees you want to provide for
reproducibility. As I say in the pr, if the seed should guarantee that you
get the same data in the same order, even for different models, then you
need this. Otherwise you're of course right.

It's of course your call, but the test provided fails against master.



On Mon, Mar 7, 2022, 4:46 AM Sylvain Gugger ***@***.***>
wrote:

> No. As mentioned in the other issues, this code is intended to be this
> way. The torch RNG has been seeded so this is completely deterministic.
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/huggingface/transformers/pull/15961#issuecomment-1060650001>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAACLIJ6VJKQGDR4VNWCJJDU6X3BLANCNFSM5QCMI7JA>
> .
> Triage notifications on the go with GitHub Mobile for iOS
> <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
> or Android
> <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
>
> You are receiving this because you authored the thread.Message ID:
> ***@***.***>
>


Oh, I read too fast and didn't catch your whole point. Thanks for expanding :-)
How about we turn this on and off with a flag? Since the `seed` is set by default, I'm afraid this will hinder users launching the train method several times (they'll get the same shuffle then, instead of different ones).

That makes sense to me! How about:

* we add a new flag called "sampler_seed" (or "data_seed" or whatever you
want) which defaults to None, and
* we use the old behavior if it's None and otherwise use sampler_seed for
this generator and for all the other seeds in that method otherwise.


On Mon, Mar 7, 2022 at 7:54 AM Sylvain Gugger ***@***.***>
wrote:

> Oh, I read too fast and didn't catch your whole point. Thanks for
> expanding :-)
> How about we turn this on and off with a flag? Since the seed is set by
> default, I'm afraid this will hinder users launching the train method
> several times (they'll get the same shuffle then, instead of different
> ones).
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/huggingface/transformers/pull/15961#issuecomment-1060838414>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAACLILPQPQTGMDV4GY35D3U6YRDTANCNFSM5QCMI7JA>
> .
> Triage notifications on the go with GitHub Mobile for iOS
> <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
> or Android
> <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
>
> You are receiving this because you authored the thread.Message ID:
> ***@***.***>
>


Sounds good to me!

Fwiw, in messing with our downstream project mistral, I discovered another
place where the existing behavior could prove problematic, though
realistically mostly in unit tests

```python
trainerA = Trainer(args=TrainingArguments(seed=7), ...)
trainerB = Trainer(args=TrainingArguments(seed=7))
loaderA = trainerA.get_train_dataloader()
loaderB = trainerB.get_train_dataloader()
```

loaderA and loaderB will be initialized with different seeds because the
calls to set_seed are in the constructors for trainerA and trainerB. This
seems surprising and undesirable to me.

Happy to stick with the plan, but if that changes your mind about the
default, please let me know!

On Mon, Mar 7, 2022 at 10:14 AM Sylvain Gugger ***@***.***>
wrote:

> Sounds good to me!
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/huggingface/transformers/pull/15961#issuecomment-1060983404>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAACLIIK6IOJE7KF6D6LMU3U6ZBRNANCNFSM5QCMI7JA>
> .
> Triage notifications on the go with GitHub Mobile for iOS
> <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
> or Android
> <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
>
> You are receiving this because you authored the thread.Message ID:
> ***@***.***>
>


not sure what's going on with the doc build. it looks unrelated to me, but please let know if I'm wrong.

Otherwise, I made data_seed along the lines discussed, so I think it's RFAL.

Thanks again for your PR!

Thanks for reviewing and accepting!