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

[LayoutLMv3] Add TensorFlow implementation #18678

Merged
merged 259 commits into from Aug 30, 2022

Conversation

ChrisFugl
Copy link
Contributor

@ChrisFugl ChrisFugl commented Aug 18, 2022

Adds a TensorFlow implementation of LayoutLMv3.

We have TensorFlow weights available that we have tested are able to produce the same results as the PyTorch models at microsoft/layoutlmv3-base and microsoft/layoutlmv3-large. Can you help us upload the TF weights to those locations?

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you read the contributor guideline,
    Pull Request section?
  • Was this discussed/approved via a Github issue or the forum? Please add a link
    to it if that's the case.
  • Did you make sure to update the documentation with your changes? Here are the
    documentation guidelines, and
    here are tips on formatting docstrings.
  • Did you write any new necessary tests?

@LysandreJik @NielsRogge

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Aug 18, 2022

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

ChrisFugl and others added 20 commits August 30, 2022 09:43
Co-authored-by: amyeroberts <22614925+amyeroberts@users.noreply.github.com>
Co-authored-by: amyeroberts <22614925+amyeroberts@users.noreply.github.com>
Co-authored-by: amyeroberts <22614925+amyeroberts@users.noreply.github.com>
Co-authored-by: amyeroberts <22614925+amyeroberts@users.noreply.github.com>
Co-authored-by: Joao Gante <joaofranciscocardosogante@gmail.com>
Co-authored-by: amyeroberts <22614925+amyeroberts@users.noreply.github.com>
@ChrisFugl
Copy link
Contributor Author

ChrisFugl commented Aug 30, 2022

@ChrisFugl the underlying issue is fixed -- can you rebase the PR please? 🙏

We should be able to merge the PR after that 🎉

@gante I followed your instructions on how to rebase, but I am afraid that it messed things up a bit. There are now 45 files changed in this PR - most of which this PR haven't even touched upon.

I resolved all conflicts while rebasing by consistently accepting the incoming changes. The tests still pass.

I think the problems with rebasing must be because I merged upstream/main into this PR prior to the first rebase.

Sorry about the rebasing issues. I am not sure what the right course of action is now?

@gante
Copy link
Member

gante commented Aug 30, 2022

@ChrisFugl can you try the solutions from this StackOverflow page?

The commits will be squashed at merge time, so everything will be fine as long as the diff we see here is correct :)

@ChrisFugl ChrisFugl changed the base branch from main to api_big2 August 30, 2022 09:21
@ChrisFugl ChrisFugl changed the base branch from api_big2 to main August 30, 2022 09:21
@ChrisFugl
Copy link
Contributor Author

@gante Ah that was simple :) Thanks! Should be all good now.

@gante
Copy link
Member

gante commented Aug 30, 2022

@ChrisFugl Fantastic, I'll merge the PR now! 🧡

Thank you so much for participating through the whole process, even with all the hiccups. LayoutLMv3 is a very useful model, I'm confident TF users will deeply appreciate your contribution 🤗

There are two post-merge action points. As always, a helping hand is appreciated (but we will do them otherwise):

  • Find and fix the tokenizer problem that is preventing the conversion of microsoft/layoutlmv3-base-chinese (and convert the model)
  • [EDIT: we are going to write about it today, ~6pm CEST] Announce the TF model on social media (twitter and LinkedIn)! Unannounced changes are changes that most users will miss. [EDIT2: shared here -- https://twitter.com/joao_gante/status/1564650841688006656 ; https://www.linkedin.com/posts/gante_tensorflow-ai-microsoft-activity-6970415100406951936-dywm?utm_source=share&utm_medium=member_desktop]

@gante gante merged commit de8548e into huggingface:main Aug 30, 2022
@ChrisFugl
Copy link
Contributor Author

Thank you @gante - and for your help with getting the PR merged in 😁 Super nice to get it merged in.

I can give a shot at the tokenizer problem 👌

Happy to help with sharing as well. Already shared the LinkedIn post. We will probably also do some more spreading of the news in my team :)

oneraghavan pushed a commit to oneraghavan/transformers that referenced this pull request Sep 26, 2022
Co-authored-by: Esben Toke Christensen <esben.christensen@visma.com>
Co-authored-by: Lasse Reedtz <lasse.reedtz@visma.com>
Co-authored-by: amyeroberts <22614925+amyeroberts@users.noreply.github.com>
Co-authored-by: Joao Gante <joaofranciscocardosogante@gmail.com>
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.

None yet

8 participants