[trainer] `model` argument is not the same depending on n_gpus

[stas00]

Extracting the discussion from #8716

Summary of the issue: prediction_step() has a model argument which is a normal model with n_gpu < 2, and a wrapped DataParallel model with n_gpu > 1. So the API suffers from ambiguity here.

The user has to really use self.model to be able to call methods like model.config() or model.generate(), which can't be called on the wrapped model. But it's very likely they will use model instead since it'll act like self.model unless under multi_gpu. And why do we even have that model argument then?

Possible solutions discussed:

1. monkeypatch torch.nn.DataParallel to expand its API to support all the methods of the original model transparently by installing a catch all __getattr__ and remap all the failed method look ups to delegate to self.module.

2. not to call the function argument model anymore, since it isn't under multi gpu, but is something else.

3. remove the model argument completely + document to always use self.model - currently in seq2seq_trainer.py once we switch to self.model, prediction_step() no longer needs model as an argument (but is it always the case?)

4. pass self.model as the model arg, and making the wrapped model available via self.wrapped_model if the user needs it.

Summary of discussion around proposed solutions:

1. too magical

2. proposed calling it wrapped_model, but it's just as confusing since most of the time it's not.

3. need to check whether wrapped model is every needed inside user functions.

4. was not discussed yet

@sgugger, @LysandreJik

[sgugger]

Note that this all internal and a user only interacts/get used with that if they subclass Trainer and override the prediction_step method. I would keep it simple since it's touching a small part of our users that should be experienced enough to be able to read the docstrings, and just detail in the docstrings with a proper warning what this model argument represents.

I can also live with 4 if it's the solution selected.

[stas00]

As I didn't participate in the design of Trainer, and I don't know whether it's meant to be easily sub-classable or not - I currently can only think of some ideas and I trust you guys to choose the most suitable solution. I hope it makes sense.

[stale]

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.

[stas00]

Well, we have implemented number 4, so closing this.