[github CI] add a multi-gpu job for all example tests
Author: stas00
Pull request: yes

As discussed in https://github.com/huggingface/transformers/pull/8315#issuecomment-722398166 we now have a growing set of tests that do distributed testing and require a multi-gpu setup, and which aren't being tested at the moment by any CI.

This PR adds a new job:

* [x] run all example tests on multi-gpu

Since these are slow, I added it only to the scheduled job.

Question: Is this really needed for every step? 
```
           source .env/bin/activate
```
It's in all test steps, even when there is multiple test suites run in sequence it's re-run.

Won't it be enough to do it once in the env prep step for all steps in the same job?

@sshleifer, @LysandreJik 

Comments:
Ah, thank you for clarifying the state of things, @LysandreJik 

Some possible solutions I can think of:

1. fix the situation quickly by adding `@require_torch_non_multigpu` to any such failing tests and make a plan to port those.
2. rename all good tests to include `_multigpu` and run the test suite with `-k multigpu` - but we already have one test that can run on either 1 gpu or many so it'd exclude it despite it being just fine (which makes it is the least favorite suggestion)
3. create a file with a list of passing tests and feed it to `pytest $(cat examples/passing_multigpu_tests.txt)`, which we maintain as tests get ported and eventually remove it.

I think solution (3) is the most sensible to me. (1) will potentially sweep things under the carpet. (2) isn't great because it doesn't cover all bases.

I went ahead and created `examples/ported-multigpu-tests.txt` and changed the new job to run:
```
python -m pytest -n 1 --dist=loadfile -s --make-reports=examples_torch_multiple_gpu $(tr '\n' ' ' < examples/ported-multigpu-tests.txt)
```
I used a little `tr` to make `ported-multigpu-tests.txt` easy to maintain: 
* one entry per line - or multiple if desired
* it can take whole test files or specific tests

We also have one distributed test under `tests` - but it automatically works with either number of gpus.

Moreover, we can actually run tests from both tests suites at once - i.e. in the same `pytest` session.


> Regarding your question concerning the `source .env/bin/activate` statement, it is actually necessary to do it at all steps. It got me by surprise in the slow TF test suite where I had forgotten it and it used the system interpreter instead.

Thank you for clarifying that! I took the liberty to add a note in `.github/workflows/self-scheduled.yml` to why this is done, to save time in the future others asking the same question.

Perhaps there is a way to make it persistent across steps. A quick look landed: https://github.com/actions/virtual-environments - that is you need to use their venv and then it's persistent across steps I think.

I wish the CIs were using the same API, as currently each one re-invents the wheel. We need huggingface `ci` interface like `transformers` that supports any major CI using the same API ;)

Another small nit I'd like to suggest: the ci yml files use `multiple_gpu` and `multi_gpu`, tests use `multigpu` in one word - let's pick one of them and use it consistently?


I would definitely be in favor of adding the `require_torch_non_multigpu` decorator to these tests, as much as I would be in favor of adding a `require_torch` to all current examples that require torch (which is all of them, I believe). Right now we can't run the TF CI on examples because it fails on all examples, and while we have no immediate need to do so as there are no examples to test in TF, this is bound to change.

I think the absolute best approach would be to make those tests robust to multi-gpu, as most scripts can handle it but the tests are not designed that way. However, that would be a considerable time investiment for limited return, so I'm fine with the decorator option. I don't feel like that would be to sweep things under the carpet, so I may have misunderstood what you meant, could you elaborate on that?

Ah, I didn't know that you could leverage the GA virtual environment to keep it persistent. I currently don't have the bandwidth to look into it, but if you do, please do! Otherwise we can do this once things settle down.

I have no opinion regarding `multigpu` vs `multi_gpu`, but consistency would be most welcome. Maybe `multigpu` is a bit harder to read, so let's go the `multi_gpu` route if you agree on that?

> I would definitely be in favor of adding the `require_torch_non_multigpu` decorator to these tests, as much as I would be in favor of adding a `require_torch` to all current examples that require torch (which is all of them, I believe). 

Excellent! 

A possible next action:
1. add `require_torch` to all current examples tests
2. add `require_torch_non_multigpu` to unported/untested under multi-gpu examples tests
3. make the multigpu CI job run on `examples` w/o specifying test names

> I think the absolute best approach would be to make those tests robust to multi-gpu, as most scripts can handle it but the tests are not designed that way. However, that would be a considerable time investiment for limited return, so I'm fine with the decorator option. I don't feel like that would be to sweep things under the carpet, so I may have misunderstood what you meant, could you elaborate on that?

When I used the idiom "sweeping under the carpet", my concern with `require_torch_non_multigpu` was that it was actually added to decorate tests that by design shouldn't run on multi-gpu, but here we use it as a band-aid, and then we won't be able to quickly tell which tests are flagged so because they are in line to be upgraded, and which are meant to not run on multigpu (can't be upgraded or if for whatever reason it needs to run on a single gpu). I hope this clarifies my concern.

How about we add a new decorator `require_torch_non_multigpu_but_fix_me` or something like that, so it's then clear this one needs to be upgraded. If I remember correctly we have a couple  `require_torch_non_multigpu` that are meant to stay so. It'd just be defined as:
```
require_torch_non_multigpu_but_fix_me = require_torch_non_multigpu
```
so it's just a visual clue for us.

Or it's possible that I'm overreacting and we somehow will keep track of what was ported and what not.

> Ah, I didn't know that you could leverage the GA virtual environment to keep it persistent. I currently don't have the bandwidth to look into it, but if you do, please do! Otherwise we can do this once things settle down.

I'm not 100% sure it is persistent as I haven't used it on GA yet. But one would think so. The current solution works for me.

> I have no opinion regarding `multigpu` vs `multi_gpu`, but consistency would be most welcome. Maybe `multigpu` is a bit harder to read, so let's go the `multi_gpu` route if you agree on that?

`multi_gpu` is perfect with me. Thank you!


I went ahead and added  `@require_torch_non_multigpu` to unported/untested under multi-gpu examples tests.

```
find examples -name "test_*" -exec perl -pi -e 's|^    def test_|    \@require_torch_non_multigpu_but_fix_me\n    def test_|' {} \;
find examples -name "test_*" -exec perl -pi -e 's|from transformers.testing_utils import |from transformers.testing_utils import require_torch_non_multigpu_but_fix_me, |' {} \;
make fixup
```

I cleaned up one test file that doesn't really use gpu and removed the new decorator from there. Plus removed it from all the tests that have been ported.

When the porting is done we can remove this decorator altogether.

On a multi-gpu machine we now have:
```
pytest examples
====================================================================== test session starts ======================================================================
platform linux -- Python 3.8.5, pytest-6.1.2, py-1.9.0, pluggy-0.13.1
rootdir: /mnt/nvme1/code/huggingface/transformers-examples-multigpu-ci
plugins: hydra-core-1.0.3, forked-1.3.0, xdist-2.1.0, instafail-0.4.2
collected 82 items

examples/test_examples.py ssssss                                                                                                                          [  7%]
examples/bert-loses-patience/test_run_glue_with_pabee.py s                                                                                                [  8%]
examples/deebert/test_glue_deebert.py s                                                                                                                   [  9%]
examples/rag/test_distributed_retriever.py sss                                                                                                            [ 13%]
examples/seq2seq/test_bash_script.py sss                                                                                                                  [ 17%]
examples/seq2seq/test_calculate_rouge.py ......                                                                                                           [ 24%]
examples/seq2seq/test_datasets.py ssssssssssssssss                                                                                                        [ 43%]
examples/seq2seq/test_finetune_trainer.py s.s                                                                                                             [ 47%]
examples/seq2seq/test_fsmt_bleu_score.py ssss                                                                                                             [ 52%]
examples/seq2seq/test_make_student.py sssss                                                                                                               [ 58%]
examples/seq2seq/test_seq2seq_examples.py ssssssssssssssssss                                                                                              [ 80%]
examples/seq2seq/test_seq2seq_examples_multi_gpu.py s.                                                                                                    [ 82%]
examples/seq2seq/test_tatoeba_conversion.py ss                                                                                                            [ 85%]
examples/seq2seq/bertabs/test_utils_summarization.py ..........                                                                                           [ 97%]
examples/token-classification/test_ner_examples.py ss                                                                                                     [100%]

========================================================== 18 passed, 64 skipped, 2 warnings in 32.04s =============
```

We can add a new task to go over tests with  `@require_torch_non_multigpu` - review those and either port them or remove the decorator, since no porting is needed there. And we can now run examples on multigpu.


So let's discuss how we designate the various single/multiple/flexible gpu decorators:

Let's focus just on pytorch tests for now

We have tests that require:
- `>= 0` gpus: `@require_torch`
- `>= 1` gpus: `@require_torch_gpu`
- `>= 2` gpus: `@require_torch_multigpu`
- `< 2` gpu: `@require_torch_non_multigpu`
-  `== 1` gpu only:  we don't have such decorator yet, so perhaps we need to add `require_torch_single_gpu` as you suggested?
- `== 0 ` gpu only:  we don't have such decorator yet, so perhaps we need to add `require_torch_non_gpu`?

Please check that I didn't miss any edge cases. The last one I'm not sure if we have such tests - probably not.

The second to last case (`== 1` gpu only) is slightly ambiguous - should we not run the test if there is more than 1 gpu? or should the test be smart and set the env so that only 1 gpu is seen? it's easy to do if it's a `n_gpu` arg, so it can just force it to `n_gpu=1` - it'd be trickier if the tested system derives this from the env - then we have to tweak `CUDA_VISIBLE_DEVICES`. we have such test already - it just sets `n_gpu=1` if there is `>= 1` - so should such test run on multi-gpu machine or not?

plus we want a distributed test that runs on a single gpu as well - @sshleifer wanted to see that it gives better results than non-distributed - so distributed shouldn't automatically mean multigpu either.

Any other nuances?


I'm not sure we have a need for the last two tests. If a test isn't working on a GPU even though it works on CPU, this is a bug. If a test isn't working on CPU even though it works on a GPU, that is a bug as well. Both of these bugs must be solved ASAP.

I think the other four decorators are enough, and cover the whole test suite. What do you think?

> If a test isn't working on CPU even though it works on a GPU, that is a bug as well. Both of these bugs must be solved ASAP.

Your comment then addresses @sshleifer question:

> should the decorator be called require_torch_single_gpu?

So you currently can't think of such situation, since it'd have to work on cpu too.

---------------------

I was thinking that perhaps there will be a test checking some feature that is only available on gpu or perhaps it's too slow to run on cpu, yet doesn't require `slow` on gpu...

I guess if we encounter such situation we can add a new decorator.

So the next stage could be: 
1) to make an issue that tracks a list of tests that need to be ported to multigpu
2) invite those in charge of the different sections of `examples` to review the tests that they maintain, remove this fix_me decorator if it's not needed and otherwise add it to the issue in #1 (or perhaps list all the available tests right away and check off what still needs to be worked on)
3) port the tests that need to be ported 
   - mechanically it is just a few lines of code to add and update the issue - but it's the best done by those who wrote/maintain the tests since they know the nuances and will see that the tests are still valid 
   - the main issue I noticed with porting to distributed is that the tests then need either more data or more iterations or a different learning rate. And to support 3+ gpus it really has to have some kind of multiplier so as n_gpus grows the number of data needs to grow proportionally. 

or we can just leave it as is and port as devs choose to/find need to...