Skip to content

Conversation

@younesbelkada
Copy link
Contributor

What does this PR do?

This PR fixes BLIP doctest.

Link to failing job: https://github.com/huggingface/transformers/actions/runs/3964164193

The docstring of the forward method of BlipForQuestionAnswering has been corrected to educate users on how to correctly use this module after #21021 being merged.
The logic now of the forward method is pretty much similar to T5.

cc @ydshieh 💯

@younesbelkada younesbelkada requested a review from ydshieh January 20, 2023 16:44
@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Jan 20, 2023

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

Copy link
Collaborator

@ydshieh ydshieh left a comment

Choose a reason for hiding this comment

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

LGTM 💯 with -1 --> I want to have ♾️ cats instead of just 2 😹

"Either `decoder_input_ids` or `labels` should be passed when calling `forward` with"
" `BlipForQuestionAnswering`. if you are training the model make sure that `labels` is passed, if you"
" are using the model for inference make sure that `decoder_input_ids` is passed."
" are using the model for inference make sure that `decoder_input_ids` is passed or call `generate`"
Copy link
Collaborator

Choose a reason for hiding this comment

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

No for the change in this PR, but if you are training should be If you are training. And the second part should be , and if you ... or . If you ...

Copy link
Collaborator

Choose a reason for hiding this comment

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

Let @NielsRogge for a second eye 👀 on this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes agree here!

@ydshieh ydshieh requested a review from NielsRogge January 20, 2023 17:12
Co-authored-by: NielsRogge <48327001+NielsRogge@users.noreply.github.com>
Copy link
Contributor

@NielsRogge NielsRogge left a comment

Choose a reason for hiding this comment

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

Thanks for fixing!

@younesbelkada younesbelkada merged commit e1cd786 into huggingface:main Jan 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