BART ce loss ignores pad_token_id instead of -100
Author: sshleifer
Pull request: yes

cc @ibeltagy who noticed this.

I think it is better to ignore pad_token_id then -100, but this is semi-breaking because old training code might have replaced pad token id with -100 in labels.

Maybe I should check for both?

Comments:
# [Codecov](https://codecov.io/gh/huggingface/transformers/pull/6804?src=pr&el=h1) Report
> Merging [#6804](https://codecov.io/gh/huggingface/transformers/pull/6804?src=pr&el=desc) into [master](https://codecov.io/gh/huggingface/transformers/commit/3cac867fac3f8717b25e3026b97b456a4e748039?el=desc) will **increase** coverage by `0.03%`.
> The diff coverage is `100.00%`.

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

```diff
@@            Coverage Diff             @@
##           master    #6804      +/-   ##
==========================================
+ Coverage   79.21%   79.25%   +0.03%     
==========================================
  Files         157      157              
  Lines       28588    28588              
==========================================
+ Hits        22646    22656      +10     
+ Misses       5942     5932      -10     
```


| [Impacted Files](https://codecov.io/gh/huggingface/transformers/pull/6804?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/transformers/modeling\_bart.py](https://codecov.io/gh/huggingface/transformers/pull/6804/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ19iYXJ0LnB5) | `95.06% <100.00%> (-0.52%)` | :arrow_down: |
| [src/transformers/modeling\_tf\_electra.py](https://codecov.io/gh/huggingface/transformers/pull/6804/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ190Zl9lbGVjdHJhLnB5) | `25.13% <0.00%> (-73.83%)` | :arrow_down: |
| [src/transformers/modeling\_tf\_openai.py](https://codecov.io/gh/huggingface/transformers/pull/6804/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ190Zl9vcGVuYWkucHk=) | `22.58% <0.00%> (-72.26%)` | :arrow_down: |
| [src/transformers/tokenization\_bart.py](https://codecov.io/gh/huggingface/transformers/pull/6804/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy90b2tlbml6YXRpb25fYmFydC5weQ==) | `42.10% <0.00%> (-57.90%)` | :arrow_down: |
| [src/transformers/trainer.py](https://codecov.io/gh/huggingface/transformers/pull/6804/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy90cmFpbmVyLnB5) | `13.21% <0.00%> (-40.45%)` | :arrow_down: |
| [src/transformers/modeling\_marian.py](https://codecov.io/gh/huggingface/transformers/pull/6804/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ19tYXJpYW4ucHk=) | `60.00% <0.00%> (-30.00%)` | :arrow_down: |
| [src/transformers/training\_args.py](https://codecov.io/gh/huggingface/transformers/pull/6804/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy90cmFpbmluZ19hcmdzLnB5) | `66.66% <0.00%> (-25.00%)` | :arrow_down: |
| [src/transformers/trainer\_utils.py](https://codecov.io/gh/huggingface/transformers/pull/6804/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy90cmFpbmVyX3V0aWxzLnB5) | `64.44% <0.00%> (-20.00%)` | :arrow_down: |
| [src/transformers/activations.py](https://codecov.io/gh/huggingface/transformers/pull/6804/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9hY3RpdmF0aW9ucy5weQ==) | `85.00% <0.00%> (-5.00%)` | :arrow_down: |
| [src/transformers/tokenization\_dpr.py](https://codecov.io/gh/huggingface/transformers/pull/6804/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy90b2tlbml6YXRpb25fZHByLnB5) | `53.15% <0.00%> (-4.51%)` | :arrow_down: |
| ... and [14 more](https://codecov.io/gh/huggingface/transformers/pull/6804/diff?src=pr&el=tree-more) | |

------

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


What do you mean default in config?
The default `ignore_index` is -100 for CrossEntropyLoss.
`pad_token_id` is overwritten by BartConfig.

Sorry mixed things up. This would make `BartForConditionalGeneration` behave differently from all the others models (all ModelForMaskedLM and T5ForConditionalGeneration use -100) so I think this is pretty breaking. Users probably have special code to changed padded token to -100, plus you may want to mask other things than the padding for loss computation (more relevant for masked LM than seq2seq but still).

I think this is some preprocessing work to do on the labels, for instance the `DataCollatorForLanguageModeling` replaces all non-masked tokens by -100 in the labels.