search

keras-team / keras-hub

Pretrained model hub for Keras 3
Apache License 2.0
804 stars 243 forks source link

mobilenet_v3 added in keras-nlp #1782

Closed ushareng closed 2 months ago

ushareng commented 3 months ago

Thanks for the PR Usha! I left a few comments. One question I have is, can we consolidate MobileNet architectures into a generic backbone like we did for Resnet here - https://github.com/keras-team/keras-nlp/blob/keras-hub/keras_nlp/src/models/resnet/resnet_backbone.py I found this blog here on the mobilenet version - https://medium.com/@pandrii000/mobilenet-architectures-17fe7406d794 The pattern we are following for KerasHub is - if the model architectures are slightly different and can be tweaked with a few args, we add them that way.

Hi @divyashreepathihalli , I went through the MobileNetV2 architecture here https://github.com/keras-team/keras/blob/v3.5.0/keras/src/applications/mobilenet_v2.py. It can be integrated with V3. I would like to clarify whether, in V2, the number of layers and corresponding parameters, which are currently hardcoded, should remain unchanged, or if we should generalize the approach to allow for these values to be derived from parameters like in v3

mattdangerw commented 3 months ago

@ushareng we should allow for the parameters to be constructor arguments reflected in the backbone's config.

Stuff like these lines should be an anti-pattern, at least for this port. One way to think about it is to imaging browsing the config.json online. https://www.kaggle.com/models/keras/bert?select=config.json It should tell you a lot about the structure of the model itself. Number of layers, number filters at different place in the model, etc.

divyashreepathihalli commented 2 months ago

@ushareng Thanks for contributing this model to KerasHub. Can you please also provide weights conversion script for this model? We will not be able to publish the model without the weights and presets added on Kaggle.