opened
RafaelWO
username_0
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
Transformer-XL: Input and labels for Language Modeling
issue
created
sgugger
username_1
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 @username_2 (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?)
comment
created
TevenLeScao
username_2
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.
comment
created
RafaelWO
username_0
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`.
comment
created
TevenLeScao
username_2
Yes, it was changed in 2.9.0. You should probably consider updating ;)
comment
created
sgugger
username_1
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.
comment
created
RafaelWO
username_0
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!
comment
closed
RafaelWO
username_0
issue
created
RafaelWO
username_0
Hello again, sorry for bothering again, but I have update my code from version 2.6 to 2.11 as @username_2 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!
comment
created
TevenLeScao
username_2
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. I'm comparing the two.
comment
created
RafaelWO
username_0
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.
comment
created
RafaelWO
username_0
Could it be that this is also related to #4826 ?
comment
created
RafaelWO
username_0
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.
comment