transformers
transformers copied to clipboard
[WIP] Add Tensorflow implementation of Efficientformer
What does this PR do?
Adding Efficientformer computer vision model in tensorflow. WIP - will tag HF folks soon but feel free to comment at any time.
Fixes # (issue)
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?
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.
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint.
cc @Rocketknight1
Hi @D-Roberts, just letting you know the TF team at Hugging Face is aware of this and definitely interested in the port! Please ping me or @gante whenever it's ready for review, or if you run into any issues while porting.
@Rocketknight1 @gante This PR is now ready for review.
cc @amyeroberts for core maintainer review as well
@Rocketknight1 @amyeroberts I addressed your comments and also submitted two PRs for the l1 and l3 weights (and tagged Rocketknight1). Let me know what's next!
@D-Roberts - that's great!
For the CI - it seems there is an issue with your CircleCI permissions, as the tests won't run. Could you try refreshing your permissions as shown here? Once all the tests are green, we'll be ready for final reviews :)
@amyeroberts Thanks for pointing out the circle ci fix. It appears that one doc test which (rightly) can't find tf weights is failing for now. I added back the from_pt in the model tests for the sake of ci tests until the tf weights get merged.
@D-Roberts Just to let you know, we've reached out to the team at Snap to ask them to merge your PRs on the EfficientFormer checkpoints. Sorry for the delay!
@D-Roberts the checkpoint PRs should be merged now. Thank you to @alanspike for the quick response!
@amyeroberts @Rocketknight1 All local tests pass with the new tf weights. The CI gets this documentation tests failing; the pt version also predicts 281 which maps to label_281 in config.
@D-roberts I think it's fine to swap those tests for just checking the actual argmax index rather than the id2label string value. Obviously the repository config doesn't actually have the id2label values set, so fixing that would require another PR to the repos.
@Rocketknight1 @amyeroberts Alright. :)
LGTM now - I'm happy to merge as soon as you and @amyeroberts are!
Hi @Rocketknight1 , I've addressed the last comments from @amyeroberts and had all tests pass. I am ready for merge whenever you are. I've just rebased to upstream and there are some unrelated ci tests failing, though last night everything was green.
@Rocketknight1 All green again. :)
@sgugger @amyeroberts @Rocketknight1 I was wondering - when do you plan a transformers release that includes this code?
@D-Roberts We release roughly once a month and are planning on releasing 4.30 later this week. If you need it right now, it's possible to install from source to have the main version too.