Issues with the EncoderDecoderModel for sequence to sequence tasks

[dbaxter240]

❓ Questions & Help

I have been attempting with various models to try to build an encoder-decoder, sequence to sequence transformer model. For the most part, I have been using BERT (bert-base-cased), but have encountered issues with various models.

The model is intended for an English to English sequence to sequence problem.

For reference, I had been trying to use the seq2seq example in this pull request as a template :

#3402

But have needed to make some modifications to it to account for other recent changes in the EncoderDecoderModel class.

I have a hit a few main issues, three are posted here. I think at least some of them are possibly bugs in the EncoderDecoderModel code.

1. A recent commit made some major changes to the forward method, and I've been hitting issues with the section that defines the decoder_outputs (around line 253 of modeling_encoder_decoder.py.) The example in the pull request I linked does not provide decoder_input_ids when setting up the model, but that is now required by this code in your recent commit. When training, I modified the code to provide decoder_token_ids as the target tokens shifted one to the right with a PAD token in front, as described in various papers. However, I don't understand why this is required when in eval mode -- shouldn't the model not have any decoder input tokens when in test/eval mode, and only be able to see what the previous tokens it actually output were? I don't understand what I'm supposed to provide as decoder_input_ids when in evaluation mode, and haven't been able to find documentation on it.

The code I'm currently using for training looks something like this :

        for step, batch in enumerate(epoch_iterator):


            # Skip past any already trained steps if resuming training
            if steps_trained_in_current_epoch > 0:
                steps_trained_in_current_epoch -= 1
                continue

            model.train()
            batch = tuple(t.to(args.device) for t in batch)
            input_ids, output_ids, input_mask, output_mask, _, decoder_ids = batch

            # add other inputs here, including kwargs
            **inputs = {"input_ids": input_ids, "attention_mask": input_mask, 'decoder_input_ids': decoder_ids}**

            # The output tuple structure depends on the model used and the arguments invoked
            # For BERT-type models, this is
            # decoder_predictions, encoded_embeddings, encoded_attention_mask = model(**inputs)
            # For GPT2-type models, this at least starts with the decoder predictions
            # See the EncoderDecoderModel class for more details
            **output = model(**inputs)**

More context is given in the linked pull request, since again this is being copied from there. The initial pull request does not provide the 'decoder_input_ids' parameter, but it seems that is now required. My code is similar in eval mode, but without decoder_input_ids, and this code fails :

**for batch in tqdm(eval_dataloader, desc="Evaluating"):
        batch = tuple(t.to(args.device) for t in batch)
        input_ids, output_ids, input_mask, output_mask, _, decoder_ids = batch
        with torch.no_grad():
            inputs = {"input_ids": input_ids, "attention_mask": input_mask}

            # The output tuple structure depends on the model used and the arguments invoked
            # For BERT-type models, this is
            # decoder_predictions, encoded_embeddings, encoded_attention_mask = model(**inputs)
            # For GPT2-type models, this at least starts with the decoder predictions
            # See the EncoderDecoderModel class for more details
            output = model(**inputs)**

This code fails in modeling_encoder_decoder, line 283 with

ValueError: You have to specify either input_ids or inputs_embeds

2. The pull request uses a GPT2 model as an example, but that no longer works because the code mentioned from Create DataParallel model if several GPUs #1 requires some parameters like encoder_hidden_states that GPT2 does not take at initialization. When I try to create a GPT2 model I get exceptions regarding this extra parameter. In other words, when I switch from a bert-bert model to a gpt2-gpt2 model, the code posted above fails in the "forward" method of the EncoderDecoderModel (line 283 of modeling_encoder_decoder) because "encoder_hidden_states" is an unexpected param for GPT2. Is this intended / is GPT2 no longer supported for an encoder decoder architecture using this code?

3. This one is just more of a general question... but since I'm posting the above 2 as issues anyways, I figured I'd add it here in case anybody can clarify and save a separate issue being created..

I believe I'm doing this part correctly, but it was not handled in the example code so want to verify if possible... For the attention mask for the decoder, during training all non-PAD tokens are expected to be unmasked, and during evaluation no mask should be provided and a default causal mask will be used, right?

@patrickvonplaten , tagging you in this issue as requested.

Thank you for your time!! Let me know if you need more code, again my code is 95% or so identical to the run_seq2seq.py example in the linked PR, just with some changes to account for recent modifications in modeling_encoder_decoder.py

[dbaxter240]

Update -- after hours working with this code, I somehow only now realized that the PR I linked had updates to modeling_encoder_decoder.py that fixed the issues I'm describing in part 2 of my issue, which is why the example works there.

I am still confused about part 1 (and 3) however, since it does not look like that PR changed anything about the input_ids for the decoder.

[patrickvonplaten]

Yeah sorry, we changed the code base quite a bit since the PR you linked. So in general at the moment GPT2 cannot be used as a decoder because it is missing cross attention layers.
The only encoder-decoder model supported atm is a Bert-2-Bert model (this also included all models that inherit from BERT though: Roberta, ...). Do you currently use a Bert-2-Bert model?

[dbaxter240]

Thanks Patrick. I did get a bert-2-bert model working for sequence to sequence but it really did not perform well on dummy tasks such as the one in the PR I linked. I am not sure I understand how a Bert-2-Bert model is supposed to work, isn't BERT an encoder architecture? How is it used as a decoder? (I was able to get the code working, but don't understand the theory behind a bert-2-bert model, and am wondering if that explains the poor performance with this model type.)

[patrickvonplaten]

Can you link your code of your working Bert-2-Bert model here? Just a link to a GitHub repo or post it in the issue here would be great :-)

[dbaxter240]

@patrickvonplaten My code was almost totally copied from that example in the pull request. I've been experimenting a bunch so it hasn't been constant, but I tried a bert2bert model again last night and while it looked like it was training properly etc, the model did not produce any results in the end.

I've pushed the code to a new repo here that you can look at https://github.com/dbaxter240/bert2bertexample

Since my original raising of this issue, I ended up cloning the transformers repo to manually make some of the changes that were in the pull request I linked. Since then, I've been able to get a GPT2 model to actually work reasonably well on the dummy problem, but Bert2Bert still fails.

The repo contains my modified copy of modeling_encoder_decoder.py so you can see what's going on. It's essentially a few of the same changes made to the file in the PR I linked.

I'm not sure if this now falls out of your realm to investigate since I've modified the source code now, but the Bert2Bert model should be working exactly as it was prior to me tweaking the source code. I've been reading into your documentation on how to use BERT as a decoder, and as far as I can tell I'm (or the existing source code is) providing the expected parameters correctly.

Thanks!

[patrickvonplaten]

Hi @dbaxter240,
Multiple bugs were fixed in #4680. Can you please take a look whether this error persists?

I think ideally you should not copy paste old encoder decoder code into another repo since the code quickly becomes outdated and is hard to debug for us. The EncoderDecoderModel is still a very premature feature of this library and prone to change quickly. It would be great if you could try to use as much up-to-date code of this library as possible.

I'm very sorry, for only finding this big bug now! It seems like you have invested quite a lot of energy into your code. I will soon (~2 weeks) open-source a notebook giving a nice example of how the EncoderDecoderModel can be leverage to fine-tune a Bert2Bert model.

Also note that this PR #3402 is rather outdated and since we don't provide EncoderDecoderModel support for GPT2 at the moment still not possible.

[dbaxter240]

@patrickvonplaten Thank you very much for your time with this!

I haven't had too much time to play with the code including your change yet, but it looks like there are some differences in the behavior, so perhaps I will have better results once I'm able to put more time into training up the model!

I think the main question/issue I'm still hitting in my limited time toying with it is my question #1 from above. A main reason behind me trying to modify the source code originally was the required parameter of either decoder_input_ids or decoder_input_embeds, and not totally understanding what to provide there (at training vs. evaluation time.)

I'd taken a hint from the PR I'd mentioned which just passed the encoder hidden states as the decoder_input_embeds, so that's what I was trying to achieve. Using the code including your change, those parameters are required again and I can't quite use that approach.

It looks like the encoder hidden states are being passed into the decoder in the EncoderDecoderModel.forward() method via the encoder_hidden_states parameter, so that looks good, but then as mentioned in question 1 I'm not sure I understand what the expected input for decoder_input_ids or decoder_input_embeds is. Is the idea that you provide decoder_input_ids as the expected output token ids (shifted right with a PAD token) during training so the model has the expected output while training, but then completely mask those tokens during evaluation so your model can't "see the answer"?

I will keep playing with it to see if I can figure that piece out, but if you have any tips or input I would greatly appreciate it!

Thank you again for your help with this!

[patrickvonplaten]

Maybe #4647 (comment) might help as well here

[dbaxter240]

@patrickvonplaten Thanks Patrick, that did clear up a fair bit for me (especially regarding not needing to shift the tokens, but I'm still not sure I understand the answer to my main question in #1 above.

In the issue you linked, you are providing the target sequence (converted to token ids) as decoder_input_ids for training. This makes sense to me, since the underlying code is shifting the tokens right by one for us. What I still don't understand is what to provide as the decoder_input_ids when doing evaluation.

1. If I do that same thing with my test set (feed the target sequence as decoder_input_ids), then I'm just basically feeding the answer to my model. I tested that it is in fact "cheating" and using this information by putting some crazy things in my test set which the model managed to classify accurately (it definitely should not have been able to.)

2. If I instead feed the source sequence converted to token ids during evaluation (as I've seen in some documentation) then I'm giving my model different information during training and evaluation.

3. If I try to not provide any decoder_input_ids during evaluation (after calling model.eval() ), then I get a "ValueError: You have to specify either input_ids or input_embeds."

My expectation was that during training, I would feed it the target sequence as decoder_input_ids and then during evaluation, I would not input decoder_input_ids and the model would only use the previous tokens it had generated. If I provide the target sequence as decoder_input_ids during training, what am I supposed to be providing as decoder_input_ids during evaluation?

Thank you again for your help!

[dbaxter240]

Disregard the above comment -- as you hinted above I was confusing myself by looking at some outdated examples :)

I'm now generating my predictions with

decoder_predictions = model.generate(input_ids, decoder_start_token_id=model.config.decoder.pad_token_id)

I haven't been able to get too great of results yet, but haven't been able to dig into why with recent updates. I will be continuing to test over the next few days and will let you know if the bug fixes you mention above made the difference!

[mitesh-mutha]

I'm very sorry, for only finding this big bug now! It seems like you have invested quite a lot of energy into your code. I will soon (~2 weeks) open-source a notebook giving a nice example of how the EncoderDecoderModel can be leverage to fine-tune a Bert2Bert model.

@patrickvonplaten Any updates on the notebook or any other examples to fine-tune a Bert2Bert model? I find myself unsure of how to go about it and the examples would be a good starting point to understand the same. I have picked up some things from other issues (#4647) regarding this but not sure if I am doing the right thing.

[dbaxter240]

@mitesh-mutha For what it's worth, I was able to get a model up and running with pretty reasonable results going off of the code linked in the last comment of that work item. Not sure if you when an official example will be available, but that code helped me a lot if you haven't looked at that code much yet.

[patrickvonplaten]

@mitesh-mutha - very bad time estimation from my part again :D Next week (promise!), I will start working on notebook training / fine-tuning Bert2Bert on summarization. But the core code should not differ very much from the code I posted in the other comment.