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

Update bros checkpoint #26277

Merged
merged 3 commits into from
Sep 20, 2023
Merged

Conversation

jinhopark8345
Copy link
Contributor

@jinhopark8345 jinhopark8345 commented Sep 20, 2023

What does this PR do?

Update Bros checkpoint.

The naver-clova-ocr/bros-base-uncased checkpoint has bbox_projection layer in BrosEmbeddings class but this layer is moved to BrosBboxEmbeddings class. And users are expected to use jinho8345/bros-base-uncased checkpoint instead of naver-clova-ocr/bros-base-uncased.

  • naver-clova-ocr/bros-base-uncased : original pretrained checkpoint from naver-clova-ocr
  • jinho8345/bros-base-uncased : weights renamed version from naver-clova-ocr/bros-base-uncased using conversion script

Before submitting

Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.

@ydshieh

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.

Thank you @jinhopark8345 🤗

@HuggingFaceDocBuilderDev

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

@ydshieh ydshieh merged commit 37c205e into huggingface:main Sep 20, 2023
@ArthurZucker
Copy link
Collaborator

Is there a reason why we did not go with creating new repos with naver-clova-ocr/bros-base-uncased-hf instead of a custom repo?

@jinhopark8345
Copy link
Contributor Author

Is there a reason why we did not go with creating new repos with naver-clova-ocr/bros-base-uncased-hf instead of a custom repo?

No reason at all. I also considered that it would be more reliable to use checkpoint that huggingface controls.

@ArthurZucker
Copy link
Collaborator

I'll ask for the models to be moved if that is okay with you?

@ydshieh
Copy link
Collaborator

ydshieh commented Sep 20, 2023

I am not sure if @jinhopark8345 can move repo to naver-clova-ocr. Even for us, we will need naver-clova-ocr to approve I think.

Sorry merged this PR without waiting core maintainers' approval.

@jinhopark8345
Copy link
Contributor Author

@ArthurZucker I can not move the repo to naver-clova-ocr as @ydshieh assumed. And I am sorry that I didn't mention about my concern earlier, @ydshieh.

@ArthurZucker
Copy link
Collaborator

I'll ask for the checkpoints to be move then 😉

parambharat pushed a commit to parambharat/transformers that referenced this pull request Sep 26, 2023
* fix bros integration test

* update bros checkpoint
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