[WIP] feat(wandb): add logging to TFTrainer
Author: borisdayma
Pull request: yes

Bring logging feature parity from `Trainer` to `TFTrainer`.
Code has been refactored to share logging utilities.

Comments:
Note that there are still a few differences.

For example `TFTrainer` uses `args.eval_steps`.
It could make sense to refactor training args so both classes share the same ones when possible.

Here is an example run with `TFTrainer` on MRPC.

![image](https://user-images.githubusercontent.com/715491/83705691-3db64e00-a5db-11ea-93d1-c51a83559997.png)

[Link to W&B run](https://app.wandb.ai/borisd13/huggingface/runs/1zngxzw0?workspace=user-borisd13)

# [Codecov](https://codecov.io/gh/huggingface/transformers/pull/4756?src=pr&el=h1) Report
> Merging [#4756](https://codecov.io/gh/huggingface/transformers/pull/4756?src=pr&el=desc) into [master](https://codecov.io/gh/huggingface/transformers/commit/e80d6c689bd62f805a5c8d77ec0cc3b09f240d14&el=desc) will **decrease** coverage by `0.00%`.
> The diff coverage is `39.24%`.

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

```diff
@@            Coverage Diff             @@
##           master    #4756      +/-   ##
==========================================
- Coverage   77.10%   77.09%   -0.01%     
==========================================
  Files         128      128              
  Lines       21723    21734      +11     
==========================================
+ Hits        16749    16756       +7     
- Misses       4974     4978       +4     
```


| [Impacted Files](https://codecov.io/gh/huggingface/transformers/pull/4756?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/transformers/trainer\_tf.py](https://codecov.io/gh/huggingface/transformers/pull/4756/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy90cmFpbmVyX3RmLnB5) | `19.06% <9.67%> (+0.02%)` | :arrow_up: |
| [src/transformers/trainer\_utils.py](https://codecov.io/gh/huggingface/transformers/pull/4756/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy90cmFpbmVyX3V0aWxzLnB5) | `69.76% <56.66%> (-30.24%)` | :arrow_down: |
| [src/transformers/trainer.py](https://codecov.io/gh/huggingface/transformers/pull/4756/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy90cmFpbmVyLnB5) | `39.10% <61.11%> (-0.15%)` | :arrow_down: |
| [src/transformers/file\_utils.py](https://codecov.io/gh/huggingface/transformers/pull/4756/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9maWxlX3V0aWxzLnB5) | `73.38% <0.00%> (-0.41%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/huggingface/transformers/pull/4756?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/4756?src=pr&el=footer). Last update [e80d6c6...28342e9](https://codecov.io/gh/huggingface/transformers/pull/4756?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


@jplu while I'm doing it, is the `step` variable from `_prediction_loop` used or can I remove it?

It is used for logging.

Ping @julien-c as there are some changes in the PT trainer.

> It is used for logging.

Ok, I don't see it being used anywhere in that function…

Hummm indeed... Can you still keep it, I will review this later to better check that part :)

I should have addressed your comments. I moved tensorboard specific logging directly within specific trainers and call it from shared `log_metrics`.
It could have also been the opposite with a public `trainer.log` method that calls `log_metrics` for everything non-tensorboard (wandb and stdout) but I thought this separation would be less obvious to users.

Let me know if you have any other suggestions.

The main differences now between the 2 trainers are the use of `args.debug` and `args.eval_steps` in `TFTrainer`.

Let me know if anything else is needed

The last CI error seems unrelated to this PR.

Ok it is fine for me for the TF part. I let @julien-c to review.

Can I have any feedback on any possible changes still required?
This is a pretty large refactor of logging so it's hard to keep up to date with the repo ;)

I merged master. Main change is logging for wandb & Tensorboard is applied only for world master now.
Need to figure out if the same should apply for `TFTrainer`. I left a comment on the code.

Once we finalize this, I'll run again logging with both `Trainer` and `TFTrainer` to make sure everything works and later I'll work on tests in a follow-up PR.

Thanks! Looks ok, about the world master there is no need for the TF part.

Based on above comments, I can just put all the wandb logic separately in their respective Trainer's (and not use `trainer_utils` anymore).

Could you confirm this is the way to go @jplu @julien-c 

I made a new PR as I understood you prefer not to refactor logging into a single file like I did here.
Feel free to close this one if my understanding was correct.

Obsolete PR