Skip to content

Conversation

SunMarc
Copy link
Member

@SunMarc SunMarc commented Sep 11, 2025

What does this PR do?

This PR fixes Florence 2 model kwargs issue with num_items_per_batch. Since BART decoder don't take any kwargs, we shouldn't pass them in forward, otherwise we get an error.
TypeError: BartDecoder.forward() got an unexpected keyword argument 'num_items_in_batch'

cc @merveenoyan

Copy link
Contributor

[For maintainers] Suggested jobs to run (before merge)

run-slow: florence2

@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@merveenoyan
Copy link
Contributor

I tested it and it fixes my issue, thanks a ton!

Copy link
Contributor

@vasqu vasqu left a comment

Choose a reason for hiding this comment

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

Wouldn't it be better to allow loss kwargs on Bart (top level)?

The reason Bart doesn't have the kwargs support is that while the attentions are already refactored. there are 3 different attention flavors (encoder, decoder, cross) which needs a proper separation (not covered by our current way of things). And this issue seems specific to just the loss calculation so should be fine to add kwargs on the user facing modules and type it properly.

@SunMarc
Copy link
Member Author

SunMarc commented Sep 12, 2025

I agree with what you said @vasqu. Maybe we can first merge this PR to unblock @merveenoyan and do that in a follow-up PR as this PR doesn't create any regression?

Copy link
Contributor

@vasqu vasqu left a comment

Choose a reason for hiding this comment

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

Gotcha, please update this later then 🤗 since it doesn't have attention backend support (yet), it should be fine

@SunMarc SunMarc merged commit ada64ce into main Sep 15, 2025
18 checks passed
@SunMarc SunMarc deleted the fix-florence2 branch September 15, 2025 09:05
ErfanBaghaei pushed a commit to ErfanBaghaei/transformers that referenced this pull request Sep 25, 2025
vijayabhaskar-ev pushed a commit to vijayabhaskar-ev/transformers that referenced this pull request Oct 2, 2025
yuchenxie4645 pushed a commit to yuchenxie4645/transformers that referenced this pull request Oct 4, 2025
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.

5 participants