[WIP] Trainer supports all Datasets as train_dataset, with/without __len__ #5990
A first PR.

Passed:
- `make test`
- `make style`
- `make quality`

Modifies:
- trainer.py: fixes issue
- test_trainer.py: calls `Trainer.train()`

It fixes only the case where the TRAINING dataset has not the `__len__` method.

The distinction is not between `Dataset` and `IterableDataset`, but between objects that are instances of class where `__len__` is implemented or not. This is pointed in [pytorch source](https://pytorch.org/docs/stable/_modules/torch/utils/data/dataset.html#Dataset), implementation of `__len__` is up to the user.
The test is therefore: `isinstance(dataset, collections.Sized)`

NB: fixing for EVAL and TEST dataset will require more code refactor: all get funneled to `Trainer._prediction_loop()` without keeping track of whether it is EVAL or TEST, which makes the usage of `TrainingArguments.eval_steps` impossible to assume. (not to mention, there is no `test_steps` field in `TrainingArguments`)

Not all datasets have an implementation of `__len__` method, therefore the trainer should not assume it is available.

The use case is:
- Dataset with `__len__`: use `num_train_epochs` or `max_steps` to specify how long should training run
- Dataset without `__len__`: use only `max_steps`

The limitation is still valid on the EVAL / TEST dataset who still has to implement `__len__`
> Note that there is some moving around of the code in Trainer coming in #5982 (I'll probably merge it today) so you may need to adapt a bit the code.

I'll wait for the merge of #5982 and introduce the fix for #5990 after.

> Note that there is no test_steps field since the evaluation is supposed to be complete on the test set (users can always pass along a shorter dataset if they want).

I see. 
At the moment, the functionality is that it will refuse a dataset that does not implement `__len__`, whether it is the EVAL dataset or the TEST dataset.

<|||||># [Codecov](https://codecov.io/gh/huggingface/transformers/pull/5995?src=pr&el=h1) Report
> Merging [#5995](https://codecov.io/gh/huggingface/transformers/pull/5995?src=pr&el=desc) into [master](https://codecov.io/gh/huggingface/transformers/commit/c69ea5efc4eac65b183e8d07b1bf91d20bbe0c8c&el=desc) will **increase** coverage by `0.40%`.
> The diff coverage is `78.57%`.

[![Impacted file tree graph](https://codecov.io/gh/huggingface/transformers/pull/5995/graphs/tree.svg?width=650&height=150&src=pr&token=9qOlN6Hb1c)](https://codecov.io/gh/huggingface/transformers/pull/5995?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master    #5995      +/-   ##
==========================================
+ Coverage   78.50%   78.90%   +0.40%     
==========================================
  Files         146      146              
  Lines       26249    26264      +15     
==========================================
+ Hits        20606    20723     +117     
+ Misses       5643     5541     -102     
```


| [Impacted Files](https://codecov.io/gh/huggingface/transformers/pull/5995?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/transformers/trainer.py](https://codecov.io/gh/huggingface/transformers/pull/5995/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy90cmFpbmVyLnB5) | `60.63% <78.57%> (+19.75%)` | :arrow_up: |
| [src/transformers/generation\_tf\_utils.py](https://codecov.io/gh/huggingface/transformers/pull/5995/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9nZW5lcmF0aW9uX3RmX3V0aWxzLnB5) | `86.21% <0.00%> (+0.75%)` | :arrow_up: |
| [src/transformers/tokenization\_bert.py](https://codecov.io/gh/huggingface/transformers/pull/5995/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy90b2tlbml6YXRpb25fYmVydC5weQ==) | `92.23% <0.00%> (+0.91%)` | :arrow_up: |
| [src/transformers/optimization.py](https://codecov.io/gh/huggingface/transformers/pull/5995/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9vcHRpbWl6YXRpb24ucHk=) | `97.36% <0.00%> (+1.31%)` | :arrow_up: |
| [src/transformers/modeling\_auto.py](https://codecov.io/gh/huggingface/transformers/pull/5995/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ19hdXRvLnB5) | `77.90% <0.00%> (+3.48%)` | :arrow_up: |
| [src/transformers/training\_args.py](https://codecov.io/gh/huggingface/transformers/pull/5995/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy90cmFpbmluZ19hcmdzLnB5) | `86.73% <0.00%> (+6.12%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/huggingface/transformers/pull/5995?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/huggingface/transformers/pull/5995?src=pr&el=footer). Last update [c69ea5e...4066671](https://codecov.io/gh/huggingface/transformers/pull/5995?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
<|||||>Hi,

- Merged to include all changes from #5982
- Now accepts `IterableDataset` for any dataset (TRAIN, EVAL, TEST)
- Displays information (how many steps, etc...)
- `test_trainer.py` includes an end-to-end test of `train()` and `predict()`

It fixes the issue #5990 with my code.

Checklist:
- `make test` is positive (all passed, no failed)
- `make style` and `make quality`

Looking forward to review.

I'm still unsure about how to test what the dataset is:
- the  type hinting says `train_dataset: Dataset`
- `pytorch` indicates it is good practice to implement `__len__` on Map-Style dataset, but in the code there is no way this could be enforced
- `pytorch` relies only on the logic: user will inherit `Dataset` and implement `__len__` or user will inherit `IterableDataset` and not implement `__len__`. In `Dataloader` every time there is a doubt it is checking if the object is an instance of `Dataset` or `IterableDataset`
- in my code, I followed my first rule: either the object has `__len__` or it does not. 

Still wondering if I should code it `pytorch`-style and trust the given answer blindly, or keep it the way it is done, which is a bit more paranoid ?
Any comment appreciated.<|||||>- the test is whether a `Dataset` object has `__len__` or not
- iterable dataset is OK for training,  __only__ if `max_steps` has a strictly positive value
- iterable dataset is not acceptable for evaluation or prediction

The confusion about `eval_steps` has been purged.

The test `test_trainer_iterable_dataset` in `test_trainer.py` has been extended to check for corner cases and associated exceptions. Only the exception type is checked, not exception message.

Checklist:
- `make test` passed (no fail)
- `make style`
- `make quality`

Looking forward to review.<|||||>Nitpicking is fine in my book.

- removed redundant test on dataset (eval / test)

Checklist:
- `make test` passed (no fail)
- `make style`
- `make quality`

Up for review again.<|||||>This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
<|||||>Hi, is this PR still being reviewed? I would like to use `Trainer` with an `IterableDataset` and this looks like exactly what's needed to make that happen. If you have time, I would greatly appreciate this PR to get into the next version :) thank you!<|||||>I realize a part has been merged, but not everything.<|||||>And I can't seem to find a way to re-open this PR. So I guess, I should open a new one, and link to this one...<|||||>@carson-sestili The PR #7858 has been merged to master and fixes the bug.
You can already use it by installing from source.
<|||||>@j-rossi-nl Thank you very much!