Skip to content

[CLIPSeg] Make interpolate_pos_encoding default to True#34419

Merged
NielsRogge merged 6 commits intohuggingface:mainfrom
NielsRogge:feature/undo_32600
Oct 31, 2024
Merged

[CLIPSeg] Make interpolate_pos_encoding default to True#34419
NielsRogge merged 6 commits intohuggingface:mainfrom
NielsRogge:feature/undo_32600

Conversation

@NielsRogge
Copy link
Copy Markdown
Collaborator

@NielsRogge NielsRogge commented Oct 25, 2024

What does this PR do?

This PR fixes #34415 by making interpolate_pos_encoding default to True.

@NielsRogge NielsRogge mentioned this pull request Oct 25, 2024
4 tasks
@HuggingFaceDocBuilderDev
Copy link
Copy Markdown

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.

@manuelsh
Copy link
Copy Markdown
Contributor

Please see my comment #34415 (comment) before merging.

@NielsRogge NielsRogge changed the title [CLIPSeg] Remove interpolate_pos_encoding [CLIPSeg] Make interpolate_pos_encoding default to True Oct 27, 2024
@NielsRogge
Copy link
Copy Markdown
Collaborator Author

I've updated my PR description based on your comments

Comment thread src/transformers/models/clipseg/modeling_clipseg.py
Copy link
Copy Markdown
Collaborator

@ArthurZucker ArthurZucker left a comment

Choose a reason for hiding this comment

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

LGTM thanks, not sure why one of the integration test is gone, congrats on getting the old logits back! 🥳

@NielsRogge
Copy link
Copy Markdown
Collaborator Author

NielsRogge commented Oct 31, 2024

I just reverted to the interpolation which is used by all other models, so the logits diff remains (edit: it looks like it has to do with my local setup)

@NielsRogge NielsRogge merged commit df8640c into huggingface:main Oct 31, 2024
BernardZach pushed a commit to BernardZach/transformers that referenced this pull request Dec 5, 2024
…34419)

* Remove interpolate_pos_encoding

* Make fixup

* Make interpolate_pos_encoding default to True

* Reuse existing interpolation

* Add integration test
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.

ClipSeg broken

4 participants