transformers icon indicating copy to clipboard operation
transformers copied to clipboard

[WIP] Add Tensorflow implementation of Efficientformer

Open D-Roberts opened this issue 2 years ago • 5 comments

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.

D-Roberts avatar Apr 06 '23 13:04 D-Roberts

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

cc @Rocketknight1

sgugger avatar Apr 06 '23 15:04 sgugger

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 avatar Apr 17 '23 16:04 Rocketknight1

@Rocketknight1 @gante This PR is now ready for review.

D-Roberts avatar May 15 '23 18:05 D-Roberts

cc @amyeroberts for core maintainer review as well

Rocketknight1 avatar May 16 '23 14:05 Rocketknight1

@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 avatar May 21 '23 22:05 D-Roberts

@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 avatar May 22 '23 14:05 amyeroberts

@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 avatar May 23 '23 01:05 D-Roberts

@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!

Rocketknight1 avatar May 24 '23 13:05 Rocketknight1

@D-Roberts the checkpoint PRs should be merged now. Thank you to @alanspike for the quick response!

Rocketknight1 avatar May 24 '23 20:05 Rocketknight1

@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 avatar May 25 '23 01:05 D-Roberts

@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 avatar May 25 '23 13:05 Rocketknight1

@Rocketknight1 @amyeroberts Alright. :)

D-Roberts avatar May 26 '23 13:05 D-Roberts

LGTM now - I'm happy to merge as soon as you and @amyeroberts are!

Rocketknight1 avatar May 26 '23 14:05 Rocketknight1

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.

D-Roberts avatar May 30 '23 15:05 D-Roberts

@Rocketknight1 All green again. :)

D-Roberts avatar May 30 '23 20:05 D-Roberts

@sgugger @amyeroberts @Rocketknight1 I was wondering - when do you plan a transformers release that includes this code?

D-Roberts avatar Jun 07 '23 11:06 D-Roberts

@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.

amyeroberts avatar Jun 07 '23 12:06 amyeroberts