Skip to content

[BLIP] fix cross attentions for BlipTextEncoder#22515

Merged
sgugger merged 1 commit into
huggingface:mainfrom
zhbh01:fix-blip-text-encoder-cross-atten
Apr 3, 2023
Merged

[BLIP] fix cross attentions for BlipTextEncoder#22515
sgugger merged 1 commit into
huggingface:mainfrom
zhbh01:fix-blip-text-encoder-cross-atten

Conversation

@zhbh01

@zhbh01 zhbh01 commented Apr 2, 2023

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes a bug in the output of the cross attentions in BlipTextEncoder

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you read the contributor guideline,
    Pull Request section?
  • Was this discussed/approved via a Github issue or the forum? Please add a link
    to it if that's the case.
  • Did you make sure to update the documentation with your changes? Here are the
    documentation guidelines, and
    here are tips on formatting docstrings.
  • Did you write any new necessary tests?

Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.

@HuggingFaceDocBuilderDev

HuggingFaceDocBuilderDev commented Apr 2, 2023

Copy link
Copy Markdown

The documentation is not available anymore as the PR was closed or merged.

@sgugger

sgugger commented Apr 3, 2023

Copy link
Copy Markdown
Collaborator

cc @ArthurZucker and @younesbelkada

@younesbelkada younesbelkada left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the PR! Could you share more details about the bug you have encountered (i.e. how to reproduce it) and how this PR is relevant to fix that bug?

@zhbh01

zhbh01 commented Apr 3, 2023

Copy link
Copy Markdown
Contributor Author

Sure, happy to provide more details. This bug is caused by the all_cross_attentions variable not properly storing the cross-attention produced by each BlipTextLayer. The variable is initialized at line 404 and returned in either line 460 or 469, but it remains unchanged between initialization and return. As a result, the forward function consistently returns an empty tuple for cross-attention.

To address this issue, I have made changes to ensure that all_cross_attentions correctly stores the cross-attention produced by each BlipTextLayer, allowing the forward function to return the appropriate cross-attention.

To reproduce the bug, please run the following snippet (the returned cross attentions will always be an empty tuple):

import torch
from PIL import Image
from transformers import BlipProcessor, BlipForQuestionAnswering

processor = BlipProcessor.from_pretrained("Salesforce/blip-vqa-base")
model = BlipForQuestionAnswering.from_pretrained("Salesforce/blip-vqa-base").to("cuda")
model.text_encoder.config.output_attentions = True

img_path = "path of an image"
raw_image = Image.open(img_path).convert('RGB')

name = "cat"
question = [
    "Is there a {} in the view?".format(name),
]
inputs = processor([raw_image]*len(question), question, padding=True, return_tensors="pt").to("cuda")

vision_outputs = model.vision_model(inputs['pixel_values'])
image_embeds = vision_outputs[0]

image_attention_mask = torch.ones(image_embeds.size()[:-1], dtype=torch.long).to(image_embeds.device)

question_outputs = model.text_encoder(
    input_ids=inputs["input_ids"],
    attention_mask=inputs["attention_mask"],
    encoder_hidden_states=image_embeds,
    encoder_attention_mask=image_attention_mask,
    return_dict=True
)

# question_outputs['cross_attentions'] will always be an empty tuple
print(question_outputs['cross_attentions'])

@younesbelkada younesbelkada left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks a lot for fixing and clarifying! Your fix is the right fix

This has been never triggered on the CIs since to check for cross_attention_output existence, the Model tester needs to have an attribute is_encoder_decoder which has to be set to True. This can be addressed in a follow-up PR

Thanks for your contribution!

@younesbelkada younesbelkada requested a review from sgugger April 3, 2023 14:51
@sgugger sgugger merged commit aecbcb3 into huggingface:main Apr 3, 2023
@zhbh01 zhbh01 deleted the fix-blip-text-encoder-cross-atten branch April 3, 2023 15:01
raghavanone pushed a commit to raghavanone/transformers that referenced this pull request Apr 5, 2023
novice03 pushed a commit to novice03/transformers that referenced this pull request Jun 23, 2023
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.

4 participants