-
Notifications
You must be signed in to change notification settings - Fork 291
Add HGNetV2 to KerasHub #2293
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
Add HGNetV2 to KerasHub #2293
Conversation
@mattdangerw @divyashreepathihalli We should be able to wrap up this PR soon. I’ve made considerable progress on D-FINE that I’m eager to push to GitHub, but the volume is considerable, and I want to avoid making this PR unmanageable. With that in mind, I’ve completed the functionality tests and numerics matching in the Colab Notebook linked in the PR description, and I’ve also written a standalone example from the user’s perspective, as you requested last time along with the numerics matching to wrap up this model. |
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! Left some thoughts on the exposed API.
Keep in mind with these a key goal is to be able to hot swap out classifier model for classifier model in a high level task without changing the high level fine-tuning code. I think there's a few spots we can do that better here (in the inline comments).
@@ -0,0 +1,58 @@ | |||
# Metadata for loading pretrained model weights. | |||
backbone_presets = { | |||
"hgnetv2_b4.ssld_stage2_ft_in1k": { |
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.
do we have dots in other preset names? i've never seen it. self consistency is more important that consistency with our source for these models, so probably go dot to underscore.
also, please take a look at other preset names in keras-hub and try to be as consistent as possible. i think we just use imagenet
instead of in1k
, for example?
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.
@mattdangerw Fair. Changed dot to underscore. Regarding in1k
to imagenet
, what do you reckon we should use for the presets that underwent pretraining and fine-tuning, i.e., those that have in22k_in1k
in the name? I haven’t changed in1k
to imagenet
yet because I feel like we should discuss this first. Writing imagenet22k_imagenet1k
feels a bit too verbose, I’d love your opinion!
Great, will look into the comments, thanks for the reviews @mattdangerw! |
@mattdangerw I've incorporated the reviews and left one comment unresolved regarding the preset names + replied to each review comment for the next round and rerun the notebook to ensure nothing has broken functionality-wise. |
@mattdangerw @divyashreepathihalli Any updates on this please, thanks! Based on this work, I'm waiting to proceed with the D-FINE PR; the task model and numerics matching have also been completed for the same. |
/gemini review |
Important Installation incomplete: to start using Gemini Code Assist, please ask the organization owner(s) to visit the Gemini Code Assist Admin Console and sign the Terms of Services. |
/gemini review |
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.
Code Review
The pull request introduces the HGNetV2 model, including the backbone, classifier, preprocessor, and layers, with comprehensive tests and a conversion script. I've provided suggestions to enhance code clarity and address potential issues. Overall, it's a valuable contribution.
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
@divyashreepathihalli Reviews have been addressed, and the notebook has been re-run to verify that there are no breakages. |
* init: Add initial project structure and files * bug: Small bug related to weight loading in the conversion script * finalizing: Add TIMM preprocessing layer * incorporate reviews: Consolidate stage configurations and improve API consistency * bug: Unexpected argument error in JAX with Keras 3.5 * small addition for the D-FINE to come: No changes to the existing HGNetV2 * D-FINE JIT compile: Remove non-essential conditional statement * refactor: Address reviews and fix some nits
Description of the Change
It is an end-to-end image classification model which had to be implemented on KerasHub as a building block towards supporting D-FINE. A number of D-FINE's presets depend on derivatives of the HGNetV2Backbone, and this model sets any required infrastructure in place to serve them. Its addition unlocks and allows follow-on integration effort toward D-FINE on KerasHub. Concurrently, I am working on exploring the integration paradigm for D-FINE with HGNetV2 layers.
Closes the first half of #2271
Reference
Please read Page 15/18, Section A.1.1 of the D-FINE paper, and the HF config files to verify this point. The
"backbone": null
argument in the HuggingFace configuration translates to an HGNetV2 backbone.Colab Notebook
Usage and Numerics Matching Colab
Checklist