Trainer reported loss is wrong when using DeepSpeed and gradient_accumulation_steps > 1
Author: rfernand2
Pull request: no
Labels: name: DeepSpeed

## Environment info
- `transformers` version: 4.7.0.dev0
- Platform: Windows-10-10.0.19041-SP0
- Python version: 3.8.0
- PyTorch version (GPU?): 1.8.1 (True)
- Tensorflow version (GPU?): not installed (NA)
- Using GPU in script?: <fill in>
- Using distributed or parallel set-up in script?: no but using DeepSpeed on a single node

### Who can help
@stas00, @sgugger (trainer.py)

### See Also
https://github.com/microsoft/DeepSpeed/issues/1107

## Information

Model I am using (Roberta)

The problem arises when using:
* [ ] the official example scripts: (give details below)
* [x] my own modified scripts: (give details below)

The tasks I am working on is:
* [ ] an official GLUE/SQUaD task: (give the name)
* [ ] my own task or dataset: (give details below)
* [ ] pretraining a Language Model (wikipedia and bookcorpus datasets)

## To reproduce

Steps to reproduce the behavior:

1.  run scripts to pretrain a model with DeepSpeed on a single node with 1 GPU for N steps (gradient_accum_steps=1)
2.  run scripts to pretrain a model with DeepSpeed on a single node with 1 GPU for N steps (gradient_accum_steps=8)
3.  note that vast difference in **loss** reported on console by trainer.py

## Expected behavior

reported loss for any number of gradient_accum_steps, nodes, or GPUs should be the mean of all losses; the same order of magnitude as shown when training with gradient_accum_steps=1, on a single node, with a single GPU.


Comments:
Please note that the fix should involve ignoring the return value of `deepspeed.backward()` in this [line](https://github.com/huggingface/transformers/blob/8d171628fe84bdf92ee40b5375d7265278180f14/src/transformers/trainer.py#L1754). Or at least not updating loss with this return value since it is the scaled loss value, similar to `scaled_loss`  in this [line](https://github.com/huggingface/transformers/blob/8d171628fe84bdf92ee40b5375d7265278180f14/src/transformers/trainer.py#L1750)

Aaaah! We had two different definitions of scaled here, I know fully understand the issue. I was thinking scaled as scaled by the gradient accumulation steps factor, not scaled as scaled by the loss scaling factor. This is an easy fix to add, will do that in a bit.

> Please note that the fix should involve ignoring the return value of `deepspeed.backward()` in this [line](https://github.com/huggingface/transformers/blob/8d171628fe84bdf92ee40b5375d7265278180f14/src/transformers/trainer.py#L1754). Or at least not updating loss with this return value since it is the scaled loss value, similar to `scaled_loss` in this [line](https://github.com/huggingface/transformers/blob/8d171628fe84bdf92ee40b5375d7265278180f14/src/transformers/trainer.py#L1750)

@tjruwase, could you please review your suggestion, since I see the deepspeed code doing scaling by GAS only. Please see:

https://github.com/microsoft/DeepSpeed/blob/c697d7ae1cf5a479a8a85afa3bf9443e7d54ac2b/deepspeed/runtime/engine.py#L1142-L1143

Am I missing something?

And running tests I don't see any problem with the current code.

@stas00, you are right my suggestion here is not correct. I initially thought that deepspeed code scaling by GAS and exposing the scaled value to the client (HF) was the problem. But based yours and @sgugger findings, it seems there is nothing to do if HF is fine with `deepspeed.backward()` returning the GAS-scaled loss. 

Sounds like this issue can be closed, once @rfernand2 agrees. 

Yes, sounds good to me.

Closing as the same report on Deepspeed side has been closed https://github.com/microsoft/DeepSpeed/issues/1107