Skip to content

[Blip] Remove redundant shift right#23153

Merged
younesbelkada merged 4 commits intohuggingface:mainfrom
younesbelkada:blip-qa-loss-fix
May 19, 2023
Merged

[Blip] Remove redundant shift right#23153
younesbelkada merged 4 commits intohuggingface:mainfrom
younesbelkada:blip-qa-loss-fix

Conversation

@younesbelkada
Copy link
Contributor

@younesbelkada younesbelkada commented May 4, 2023

What does this PR do?

Fixes #23000

In fact _shift_right does not need to be called inside BlipForQuestionAnswering as the right shifting of the tokens is done directly on the text decoder as mentioned by the user. Therefore, that class will be trained to perform next-next token prediction instead of next-token prediction.

The fix is to simply remove that shift method and the call to it.

cc @sgugger

@younesbelkada younesbelkada requested a review from sgugger May 4, 2023 16:50
Copy link
Collaborator

@sgugger sgugger 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!

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented May 4, 2023

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

@younesbelkada younesbelkada merged commit 3cb9309 into huggingface:main May 19, 2023
@younesbelkada younesbelkada deleted the blip-qa-loss-fix branch May 19, 2023 17:14
@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint.

gojiteji pushed a commit to gojiteji/transformers that referenced this pull request Jun 5, 2023
* remove redundant shit right

* fix failing tests

* this time fix tests
novice03 pushed a commit to novice03/transformers that referenced this pull request Jun 23, 2023
* remove redundant shit right

* fix failing tests

* this time fix tests
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.

Possible bug in BlipForQuestionAnswering loss computation due to redundant right-shift

3 participants