Transformer-XL: Input and labels for Language Modeling
Author: RafaelWO
Pull request: no

# ❓ Questions & Help

<!-- The GitHub issue tracker is primarly intended for bugs, feature requests,
     new models and benchmarks, and migration questions. For all other questions,
     we direct you to Stack Overflow (SO) where a whole community of PyTorch and
     Tensorflow enthusiast can help you out. Make sure to tag your question with the
     right deep learning framework as well as the huggingface-transformers tag: 
     https://stackoverflow.com/questions/tagged/huggingface-transformers 
     
     If your question wasn't answered after a period of time on Stack Overflow, you
     can always open a question on GitHub. You should then link to the SO question 
     that you posted.
     -->

## Details
<!-- Description of your issue -->
I'm trying to finetune the pretrained Transformer-XL model `transfo-xl-wt103` for a language modeling task. Therfore, I use the model class `TransfoXLLMHeadModel`.

To iterate over my dataset I use the `LMOrderedIterator` from the file [tokenization_transfo_xl.py](https://github.com/huggingface/transformers/blob/5e737018e1fcb22c8b76052058279552a8d6c806/src/transformers/tokenization_transfo_xl.py#L467) which yields a tensor with the `data` and its `target` for each batch (and the sequence length).

**My question**:
Let's assume the following data with `batch_size = 1` and `bptt = 8`:

    data = tensor([[1,2,3,4,5,6,7,8]])
    target = tensor([[2,3,4,5,6,7,8,9]])
    mems # from the previous output

I currently pass this data into the model like this:

    output = model(input_ids=data, labels=target, mems=mems)

Is this correct?

I am wondering because the documentation says for the `labels` parameter:

> labels (:obj:`torch.LongTensor` of shape :obj:`(batch_size, sequence_length)`, `optional`, defaults to :obj:`None`):
            Labels for language modeling.
            Note that the labels **are shifted** inside the model, i.e. you can set ``lm_labels = input_ids``

So what is it about the parameter `lm_labels`? I only see `labels` defined in the `forward` method.

And when the labels "are shifted" inside the model, does this mean I have to pass in `data` twice (for `input_ids` and `labels`) because `labels` shifted inside? But how does the model then know the next token to predict (in the case above: `9`) ?

I also read through [this bug](https://github.com/huggingface/transformers/issues/3711) and the fix in [this pull request](https://github.com/huggingface/transformers/pull/3716) but I don't quite understand how to treat the model now (before vs. after fix). Maybe someone could explain me both versions.

Thanks in advance for some help!

<!-- You should first ask your question on SO, and only if
     you didn't get an answer ask it here on GitHub. -->
**A link to original question on Stack Overflow**: https://stackoverflow.com/q/62069350/9478384


Comments:
Hi there!

> I currently pass this data into the model like this:
> 
> ```
> output = model(input_ids=data, labels=target, mems=mems)
> ```
> 
> Is this correct?

No, this is not correct, because the labels are shifted inside the model (as the documentation suggests). This happens [here](https://github.com/huggingface/transformers/blob/ec8717d5d8f6edc2c595ff6954ffaa2078dcc97d/src/transformers/modeling_transfo_xl_utilities.py#L104) so in your example, the target vector will become
```
tensor([[3,4,5,6,7,8,9]])
```
to be matched with the predictions corresponding to
```
tensor([[1,2,3,4,5,6,7]])
```
so you'll try to predict the token that is two steps ahead of the current one.

I am guessing that `lm_labels` is a typo for `labels`, and that you should either:
- pass `labels = input_ids` as suggested by the doc string (in this case you will not compute any loss for the last prediction, but that's probably okay)
- add something at the beginning of your target tensor (anything can work since it will be removed by the shift) : `target = tensor([[42,2,3,4,5,6,7,8,9]])`

I'm still learning the library, so tagging @TevenLeScao (since he worked on the issue/PR you mentioned) to make sure I'm not saying something wrong (also, do we want to update `LMOrderedIterator` from tokenization_transfo_xl.py to return target tensors that can be used as labels?)

Ah yes that does sound like a typo from another model's convention! You do have to pass `data` twice, once to `input_ids` and once to `labels` (in your case, `[1, ... , 8]` for both). The model will then attempt to predict `[2, ... , 8]` from `[1, ... , 7]`). I am not sure adding something at the beginning of the target tensor would work as that would probably cause size mismatches later down the line.

Passing twice is the default way to do this in `transformers`; before the aforementioned PR, `TransfoXL` did not shift labels internally and you had to shift the labels yourself. The PR changed it to be consistent with the library and the documentation, where you have to pass the same data twice. I believe #4711 fixed the typo, you should be all set ! I'll also answer on StackOverflow in case someone finds that question there.

Thanks @sgugger and @TevenLeScao for your help!

@TevenLeScao 
> before the aforementioned PR, TransfoXL did not shift labels internally and you had to shift the labels yourself

So this means that  in the versions before the fix my method with shifting the labels beforehand was correct? Because I'm currently using `transformers 2.6`.

Yes, it was changed in 2.9.0. You should probably consider updating ;)

> The model will then attempt to predict `[2, ... , 8]` from `[1, ... , 7]`).

Note that if you are using the state, the memory returned is computed on the whole `[1, ... , 8]`, so you should use `[9,10,... , 16]` as your next batch.

Thanks guys!

Sorry for asking this here, but maybe one of you can help me with my workaround in issue #3554 ? That would help me a lot!

Hello again, sorry for bothering again, but I have update my code from version 2.6 to 2.11 as @TevenLeScao has suggested. Now I experience a drop in my model's performance, but I don't know why. I use the same code as before except passing `data` in twice as suggested.

I know that this can have several other reasons but I just want to know if there where other breaking changes to `TransformerXLLMHeadModel` or to the generation process?

I skipped through the changelog in the releases but could not find anything.

Thanks in advance!

Sorry, by a drop in model performance you mean the loss is worse right? I've noticed discrepancies between CMU code performance (better) and ours in the past, so maybe a bug was introduced between 2.6 and 2.11 (never used 2.6 myself). I'm comparing the two.

Well mainly I saw differences during text generation with `model.generate()` The sequences tend to be shorter and end more often with an <eos> in 2.11, where before in 2.6 they were just cut of at some point.

But I can't guarantee that there are no mistakes from my side.

Could it be that this is also related to #4826 ?

FYI: The issue regarding worse model performance on the newer version of `transformers` is solved. There were some errors on my side.

Nevertheless, I hope that the fix in the PR linked above will improve the generated texts, since I also experience low quality output despite proper finetuning.