`disable_ngram_loss` fix for prophetnet
Author: Zhylkaaa
Pull request: yes

# What does this PR do?

This PR fixes `disable_ngram_loss` behaviour for ProphetNetForConditionalGeneration and is related to #8553 

<!--
Congratulations! You've made it this far! You're not quite done yet though.

Once merged, your PR is going to appear in the release notes with the title you set, so make sure it's a great title that fully reflects the extent of your awesome contribution.

Then, please replace this with a description of the change and which issue is fixed (if applicable). Please also include relevant motivation and context. List any dependencies (if any) that are required for this change.

Once you're done, someone will review your PR shortly (see the section "Who can review?" below to tag some potential reviewers). They may suggest changes to make the code even better. If no one reviewed your PR after a week has passed, don't hesitate to post a new comment @-mentioning the same persons---sometimes notifications get lost.
-->

Fixes #8553 


## Before submitting
- [ ] This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
- [x] Did you read the [contributor guideline](https://github.com/huggingface/transformers/blob/master/CONTRIBUTING.md#start-contributing-pull-requests),
      Pull Request section?
- [ ] Was this discussed/approved via a Github issue or the [forum](https://discuss.huggingface.co/)? Please add a link
      to the it if that's the case.
- [x] Did you make sure to update the documentation with your changes? Here are the
      [documentation guidelines](https://github.com/huggingface/transformers/tree/master/docs), and
      [here are tips on formatting docstrings](https://github.com/huggingface/transformers/tree/master/docs#writing-source-documentation).
- [ ] Did you write any new necessary tests?


## Who can review?
I guess @patrickvonplaten was using this model (I saw models on hub), sorry if I am wrong, but there is no one to tag for ProphetNet




Comments:
Hey @Zhylkaaa,

Thanks a lot for your PR! @qiweizhen - could you maybe take a look and give your opinion on this PR? I don't have much experience with training ProphetNet

Hi @patrickvonplaten , thanks for informing me. It seems it's still related to the padding tokens (default -100 or **padding_idx**) which should not be calculated loss. 
In the old version code, **expend_targets**  is filled with **self.padding_idx** however in the loss function, **padding_idx** is not fed in. It results in calculating wrong loss. 

Here @Zhylkaaa set them consistent. I suggest that 1) the outside data preprocess padding function, 2) here **expend_targets** and 3) the loss function to be consistent. 

If Huggingface Transformers default uses -100 for all the NLG models padding,  then this code can be merged. If Huggingface Transformers default uses self.padding_idx for all the NLG models padding, then not merge this code, but feed **padding_idx** into the loss function.

Thanks @qiweizhen for reviewing, 
to my knowledge Huggingface Transformers use -100 to indicate ignored tokens during loss calculations, also I wanted to ask if reduction strategy is important (Transformers use reduction=`'mean'` to my knowledge, but hear it is set to `'sum'`) because for me it's not?
I also noticed that I haven't changed this behaviour in another ProphetNet model, I will examine if it's necessary and commit changes, also I will write some tests to check for this behaviour in nearest future.

> Thanks @qiweizhen for reviewing,
> to my knowledge Huggingface Transformers use -100 to indicate ignored tokens during loss calculations, also I wanted to ask if reduction strategy is important (Transformers use reduction=`'mean'` to my knowledge, but hear it is set to `'sum'`) because for me it's not?
> I also noticed that I haven't changed this behaviour in another ProphetNet model, I will examine if it's necessary and commit changes, also I will write some tests to check for this behaviour in nearest future.

Thank you for pointing out this "mean" or "sum" problem! This line of code is converted from Fairseq version ProphetNet, which use loss sum here, to be consistent with [Fairseq Transformer] (https://github.com/pytorch/fairseq/blob/v0.9.0/fairseq/criterions/label_smoothed_cross_entropy.py#L26-L27). The reason is that in the training pipeline of Fairseq, they will do the ["mean" operation in their trainer](https://github.com/pytorch/fairseq/blob/v0.9.0/fairseq/trainer.py#L429). So we return the sum loss and sample_size for Fairseq to calculate  sum loss / sample_size (mean).
So I agree here we should use "mean" as you suggested.  Thank you @Zhylkaaa !

Hi @qiweizhen, I want to verify that I should mean label smoothing loss instead of summing it to be consistent with change of reduction strategy and also should I change `non_pad_mask` to mask where I exclude -100? (You can see this changes in last commit but I just want to be sure 😁)

Also @patrickvonplaten, I have messed up with rebasing so I needed to make reset hard, is it ok or should I close this PR and open one that doesn't change commit history when I finish?)

Great PR @Zhylkaaa! I did a small refactor and fixed the test. Thanks for your help @qiweizhen