Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix text.transformer.embeddings.position_ids key error #595

Merged
merged 2 commits into from
Aug 28, 2023

Conversation

EIFY
Copy link
Contributor

@EIFY EIFY commented Aug 11, 2023

This PR is to fix #584.

After transformers==4.31, certain text transformers no longer expect text.transformer.embeddings.position_ids (huggingface/transformers#24505). The recommendation is to use from_pretrained (huggingface/transformers#25330 (comment)) but that's not an option for loading a module. So, I just explicitly identify this inconsistency and edit state_dict if necessary. Tested with both transformers==4.30.2 and transformers==4.31.

@gabrielilharco gabrielilharco merged commit 8556945 into mlfoundations:main Aug 28, 2023
5 checks passed
@usuyama
Copy link

usuyama commented Aug 28, 2023

Thank you!

Hope this PR fixes this issue in BiomedCLIP https://huggingface.co/microsoft/BiomedCLIP-PubMedBERT_256-vit_base_patch16_224/discussions/9

@EIFY EIFY deleted the fix-test branch August 28, 2023 19:43
@Adenialzz
Copy link

transformer==4.28.0 failed here with the newest open_clip version

@EIFY
Copy link
Contributor Author

EIFY commented Nov 13, 2023

@Adenialzz I can't replicate, loading eva_giant_patch14_224 with the latest main works for me. In fact, I believe transformers==4.28.0 isn't directly used at all for loading the model for inference. This PR just makes sure that the code can load newer checkpoints from transformers>=4.31.

Interpause pushed a commit to Interpause/open_clip that referenced this pull request May 23, 2024
…ns#595)

* fix create_model & test

* better fix: explained & strict
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