bugs in xlnet XLNetLMHeadModel
Author: zhangjiekui
Pull request: no
Labels: name: PyTorch

name: Core: Modeling

name: Should Fix

# 🐛 Bug

## Information

Model I am using  XLNet :

Language I am using the model on English :

The problem arises when using:
* [True ] the official example scripts: (give details below)
* [ ] 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)

## To reproduce

Steps to reproduce the behavior:

1. just deep into modeling_xlnet.py code;
2. browsing  https://github.com/huggingface/transformers/blob/master/src/transformers/modeling_xlnet.py#L1057;
3. comparing with other LMHead,such as https://github.com/huggingface/transformers/blob/master/src/transformers/modeling_gpt2.py#L603;
4.you'll found inputs and labels are not shifted.


<!-- If you have code snippets, error messages, stack traces please provide them here as well.
     Important! Use code tags to correctly format your code. See https://help.github.com/en/github/writing-on-github/creating-and-highlighting-code-blocks#syntax-highlighting
     Do not use screenshots, as they are hard to read and (more importantly) don't allow others to copy-and-paste your code.-->

## Expected behavior

<!-- A clear and concise description of what you would expect to happen. -->
the code should be refactored to the same style as other LMHeadModels
and because xlnet tokenizer appends <'sep'> , <'cls'> on the end, so the num tokens shifted should be 2, so the think the code should be: 
        if labels is not None:
            # Shift so that tokens < n predict n
            shift_logits = logits[..., :-2, :].contiguous()
            shift_labels = labels[..., 1:-1].contiguous()
            # Flatten the tokens
            loss_fct = CrossEntropyLoss()
            loss = loss_fct(logits.view(-1, logits.size(-1)), labels.view(-1))
            # loss = loss_fct(logits.view(-1, logits.size(-1)), labels.reshape(-1))
            outputs = (loss,) + outputs

## Environment info
<!-- You can run the command `python transformers-cli env` and copy-and-paste its output below.
     Don't forget to fill out the missing fields in that output! -->
     
- `transformers` version: 2.4.1
- Platform:
- Python version:
- PyTorch version (GPU?):
- Tensorflow version (GPU?):
- Using GPU in script?:
- Using distributed or parallel set-up in script?:


Comments:
I took a closer look into the XLNet model. As I understand the [paper](https://arxiv.org/pdf/1906.08237.pdf) and how the input/target lm training data is created in the code base (look [here](https://github.com/zihangdai/xlnet/blob/bbaa3a6fa0b3a2ee694e8cf66167434f9eca9660/data_utils.py#L616)), the language modelling loss is calculated using a <mask> token for specific words similar to BERT. Opposite to BERT though, the model still performs some kind of auto-regressive training as the length input_ids and labels are regressively increased over time `T`. At each training step `t` though, the `output_ids` are equal to the `input_ids`, whereas some `input_ids` are masked and the `embeddings` corresponding to the different positions `[1,T]` only see the `input_ids` and `embeddings` of certain other positions according to a random permutation (I think Fig. 4 in the paper explains it quite well). 

Long story short, in my opinion the labels should not be a shifted version of the input_ids. 
Regarding the special tokens, it's true that the `XLNetTokenizer` add two special tokens by default. I changed that in the examples provided in the `modeling_xlnet.py` file for the `XLNetModel` and `XLNetWithLMHeadModel` as those tokens were only mainly used for the two sentence input lm pre-training (similar to BERT) and might be confusing for simpler examples. 

I added an examples in PR which gives a simple example how the `XLNetWithLMHeadModel` can be used for "standard" auto-regressive pretraining. Also when looking at the function `prepare_inputs_for_language_generation()` in  `modeling_xlnet.py`, it can be seen that a <mask> token is added to the `input_ids` in order to perform language generation. This might make everything clearer as well.

Maybe @thomwolf can confirm before closing the issue? 

Thanks ! Clear, Cool!

@patrickvonplaten As an aside, are there test available per-model that check that the output of a given model is identical to the output of the original model? Like the integration tests, but where the expected output is actually the same as the original implementation?

@BramVanroy We are working on those at the moment. So far only the new models (bart & roberta) have real IntegrationTests. Most of the LMHead models have some form of Integration Test that check whether reasonable language is generated.

@patrickvonplaten Reasonable output is indeed an important aspect, but comparing with original implementations might bring discrepancies to light quickly. I am not sure how feasible that is, so it's just a thought.

I agree 100%. We compare to the original implementations as best as we can! 

@BramVanroy we compare to the original implementations when we initially convert the models. We make sure that the output of our models is the same as the output from the official models, given a small margin error. You can find an example of this in the [`convert_pytorch_checkpoint_to_tensorflow.py` script.](https://github.com/huggingface/transformers/blob/master/src/transformers/convert_pytorch_checkpoint_to_tf2.py#L351)

> @BramVanroy we compare to the original implementations when we initially convert the models. We make sure that the output of our models is the same as the output from the official models, given a small margin error. You can find an example of this in the [`convert_pytorch_checkpoint_to_tensorflow.py` script.](https://github.com/huggingface/transformers/blob/master/src/transformers/convert_pytorch_checkpoint_to_tf2.py#L351)

@LysandreJik  From the looks of this, it seems that you are comparing the output of the imported and/or mapped weights between pt and tf. But it seems that this does not cover architectural difference (correct me if I'm wrong). For instance, the recent issue of bias being counted twice wouldn't have been caught in this test, I think? But if you have some example input case and hard-code a slice of its output from the original implementation (be that tensorflow, pytorch, or something else), then you can test that the transformer implementation (architecture + weights) behave the same.

Actually, the double bias would definitely have been caught with this! We load the original models' weights onto our models and compare the output of the two models given the same input. This usually results in a tensor of size `(batch_size, sequence_length, hidden_size)` for base models or `(batch_size, sequence_length, vocab_size)` for models with an LM head (that is a lot of values!) that we each compare individually to make sure the difference is under the defined threshold.

Where our tests failed us is that we did not have integration tests for this model at the time, which is something @patrickvonplaten is doing a great job at changing :).

@BramVanroy I think you're describing what we have in e.g. https://github.com/huggingface/transformers/blob/master/tests/test_modeling_roberta.py#L322 (and @patrickvonplaten indeed added others recently)

Here `expected_slice` is the output of the original (here, fairseq) implementation. I agree that it's a good way to ensure correctness (except in cases where the original implem is "incorrect" in some way!)

See the recently merged https://github.com/huggingface/transformers/pull/3014