Avoid accessing .dataset of a DataLoader in Trainer

[sanderland]

What does this PR do?

• Respects get_train_dataloader and such, rather than going back and looking at .train_dataset or requiring attributes in the dataloader to be accessible directly.
  • This allows for overriding it by any object which implements the methods required by a DataLoader (__len__ and __iter__) without additional requirements.
  • The original motivation was to train on a multi-task dataloader which defers to multiple dataloaders.

Before submitting

• Did you read the contributor guideline,
  Pull Request section?
• Was this discussed/approved via a Github issue or the forum? Please add a link
  to it if that's the case.
  • Discussed in Confusing interaction between training dataloaders and datasets in Trainer #16388
• Did you make sure to update the documentation with your changes? Here are the
  documentation guidelines, and
  here are tips on formatting docstrings.
• Did you write any new necessary tests?

[HuggingFaceDocBuilderDev]

The documentation is not available anymore as the PR was closed or merged.

[sanderland]

@sgugger this should be ready for review.
You were right that there were a couple of more places to change, and the logic is quite inconsistent in places. I've tried to be on the defensive side in covering cases:

• dataloader.dataset can exist or not, and have a length or not
• dataloader always has a len, but it can raise an exception in fairly common cases

This implementation works for my particular case, giving the same output in training+evaluation as before, but without the really painful workarounds.

I had a look at tests and they look complicated, so I will add some after getting confirmation that this is ok otherwise.

[sgugger]

Thanks for your PR! It tries to do too much at the same time however. There is no reason to change the signature of the function get_train_dataloader so it should be left as is IMO. Even if it's a change we would like to implement, it should be done on it own separate PR.

Then there is a lot of code that could be refactored using the has_length function (and improving it a tiny bit).

Lastly, this PR breaks the current logging of number of examples, this should be fixed before we can merge it.

[sgugger]

We have a function has_length that would simplify the code greatly here, we can add the NameError inside it.

[sanderland]

Refactoring as suggested, although has_length is a bit of a confusing name for __len__ does not raise an exception"

[sgugger]

The code will error here since you're not defining num_examples anymore.

[sanderland]

num_examples was moved up inside the if statements that deal with the len/steps/size cases

[sanderland]

Note that this comment was incorrect, it would still be -1 which causes strange outputs. Have change it to make it explicit that this should be set.

[sgugger]

Thanks for adapting, I added a few comments on the tests.

[sgugger]

Thanks for implementing all the tweaks!