Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bart: check if decoder_inputs_embeds is set #13800

Conversation

silviu-oprea
Copy link
Contributor

In BartForConditionalGeneration.forward, if labels are provided,
decoder_input_ids are set to the labels shifted to the right.
This is problematic: if decoder_inputs_embeds is also set,
the call to self.model, which eventually gets to BartDecoder.forward,
will raise an error.
The fix is quite simple, similar to what is there already in
BartModel.forward. Mainly, we should not
compute decoder_input_ids if decoder_inputs_embeds is provided.

What does this PR do?

Fixes #12475

Who can review?

@patrickvonplaten

In BartForConditionalGeneration.forward, if labels are provided,
   decoder_input_ids are set to the labels shifted to the right.
   This is problematic: if decoder_inputs_embeds is also set,
   the call to self.model, which eventually gets to BartDecoder.forward,
   will raise an error.
   The fix is quite simple, similar to what is there already in
   BartModel.forward. Mainly, we should not
   compute decoder_input_ids if decoder_inputs_embeds is provided.
Copy link
Contributor

@patrickvonplaten patrickvonplaten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

@patrickvonplaten patrickvonplaten merged commit 707f7eb into huggingface:master Oct 1, 2021
stas00 pushed a commit to stas00/transformers that referenced this pull request Oct 12, 2021
In BartForConditionalGeneration.forward, if labels are provided,
   decoder_input_ids are set to the labels shifted to the right.
   This is problematic: if decoder_inputs_embeds is also set,
   the call to self.model, which eventually gets to BartDecoder.forward,
   will raise an error.
   The fix is quite simple, similar to what is there already in
   BartModel.forward. Mainly, we should not
   compute decoder_input_ids if decoder_inputs_embeds is provided.

Co-authored-by: Silviu Vlad Oprea <silviuvo@amazon.co.uk>
lapisfluvialis pushed a commit to lapisfluvialis/transformers that referenced this pull request Oct 27, 2021
In BartForConditionalGeneration.forward, if labels are provided,
   decoder_input_ids are set to the labels shifted to the right.
   This is problematic: if decoder_inputs_embeds is also set,
   the call to self.model, which eventually gets to BartDecoder.forward,
   will raise an error.
   The fix is quite simple, similar to what is there already in
   BartModel.forward. Mainly, we should not
   compute decoder_input_ids if decoder_inputs_embeds is provided.

Co-authored-by: Silviu Vlad Oprea <silviuvo@amazon.co.uk>
@silviu-oprea silviu-oprea deleted the generation-check-if-decoder-input-embeds-is-set branch December 7, 2021 12:43
@silviu-oprea silviu-oprea restored the generation-check-if-decoder-input-embeds-is-set branch December 7, 2021 12:43
Albertobegue pushed a commit to Albertobegue/transformers that referenced this pull request Jan 27, 2022
In BartForConditionalGeneration.forward, if labels are provided,
   decoder_input_ids are set to the labels shifted to the right.
   This is problematic: if decoder_inputs_embeds is also set,
   the call to self.model, which eventually gets to BartDecoder.forward,
   will raise an error.
   The fix is quite simple, similar to what is there already in
   BartModel.forward. Mainly, we should not
   compute decoder_input_ids if decoder_inputs_embeds is provided.

Co-authored-by: Silviu Vlad Oprea <silviuvo@amazon.co.uk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BartForConditionalGeneration: decoder_input_ids should not be computed if decoder_inputs_embeds is set
2 participants