-
Notifications
You must be signed in to change notification settings - Fork 279
Add DeiT Model #2203
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
base: master
Are you sure you want to change the base?
Add DeiT Model #2203
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
@divyashreepathihalli @JyotinderSingh , kindly approve the workflows. |
@Sohaib-Ahmed21 , Thanks for the PR, could you please add the |
Yeah sure, I'll do that and update the PR, thanks! |
…ollow proper ordering of preprocessing functions with addition of center_crop utility
Added the |
I’m sharing the following resources related to this PR:
|
This PR is ready for review. Kindly approve the workflows and review the PR, thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the demo notebook, added few more comments.
Also, in the colab can you assert the parameters matching between Keras and HF model.
Example, tiny variant has "params": 5524800, can you show the output where it matches with the HF params?
Thanks for the detailed review. I'll address the reviews soon.
Yes, I'll show that. |
…and adjust checkpoint_conversion script accordingly
…ckpoint_conversion script
I've addressed all reviews, kindly review the updated PR. The notebook has also been updated to include parameter verification. |
Awesome, this looks great! Thank you. |
Is the failing test related to the PR? Kindly confirm and re-run the tests if required. |
@Sohaib-Ahmed21 no the jax-gpu segfault popped up recently, it's probably related to our test environment (we haven't tracked it down yet). You can ignore. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Few misc comments, but this is looking good!
|
||
# Metadata for loading pretrained model weights. | ||
backbone_presets = { | ||
"deit-base-distilled-patch16-384_imagenet": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we use only underscore in our preset names, no mixing dashes and underscore like this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be true for the preset names here and on Kaggle, we want consistency.
@@ -73,7 +76,10 @@ def load_task(self, cls, load_weights, load_task_weights, **kwargs): | |||
cls, load_weights, load_task_weights, **kwargs | |||
) | |||
# Support loading the classification head for classifier models. | |||
if architecture == "ViTForImageClassification": | |||
if ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we could just check if ForImageClassification
is in the name here?
Add VIT based DeiT (data-efficient image transformers) model to keras-hub along with its backbone, layers, tests and checkpoint conversion.
Paper: https://arxiv.org/pdf/2012.12877
Model card: https://huggingface.co/facebook/deit-base-distilled-patch16-384