Add top 3 HF Presets for Mobilenet

[pkgoogle]

Adds these presets:

    "mobilenetv3_small_100": "timm/mobilenetv3_small_100.lamb_in1k",
    "mobilenetv3_large_100.ra_in1k": "timm/mobilenetv3_large_100.ra_in1k",
    "mobilenetv3_large_100.miil_in21k_ft_in1k": "timm/mobilenetv3_large_100.miil_in21k_ft_in1k"

We can adjust the nomenclature but this currently works.

[divyashreepathihalli]

looks good! can you also please upload the presets on Kaggle

[pkgoogle]

remaining test appears to be a timeout/network issue.

[mattdangerw]

I think these test failures are legitimate! Can you take a look? Exception encountered: __init__() missing 2 required positional arguments: 'depthwise_stride' and 'depthwise_residual' -> this is not just timeouts.

[pkgoogle]

No worries, my original comment was for the previous CI, this one is indeed legitimate (currently it seems like it's picking up an old task.json from kaggle and thus are missing the new args needed to support these latest presets).

[divyashreepathihalli]

the tests are failing since - #2108

[mattdangerw]

Sorry I missed this part, we should remove these lines. This is trying to take num_features from the backbone config and proxy it to the classifier? Why are we doing this?

We should remove these lines for sure, but not sure what to suggest instead as I don't really understand the purpose yet.

[pkgoogle]

I think it's related to the current keras saving issue actually. Basically the new presets have a different configuration for the task classifier head, essentially a different intermediate hidden dim.

keras-hub/keras_hub/src/models/mobilenet/mobilenet_image_classifier.py

Line 37 in d56ee85

filters=num_features,

So this needs to be defined on creating the keras model from the timm config. This appeared to be the cleanest way to handle it -- another option I considered was passing in the keyword like this:

keras_model = keras_hub.models.ImageClassifier.from_preset(
    "hf://" + timm_name, num_features=num_features
)

But then the user has to know to do this. Is there a better place to handle loading arbitrary task configuration for different presets?

Just to add more data, num_features is actually not a backbone config, it's a model config from timm: https://huggingface.co/timm/mobilenetv3_large_100.miil_in21k/blob/main/config.json.

[mattdangerw]

Got it thanks for the explainer!

We definitely don't want this in the base PresetLoader this would apply to all models from all sources, and num_features is a very generic name so that's a bug waiting to happen. We could add this to the TimmPresetLoader here, as it sounds like this is timm specific.

But I wonder if there's more to figure out here... Resnet has has this same num_features field in timm here, but we aren't handling it that I can see. Does this change break resnet? Do we want an update to the the base ImageClassifier class to handle this type of post-pooling convolution? Let's make sure we are doing this in a way that is consistent across our image classification models.

[pkgoogle]

Yeah I can explain why this isn't popping up anywhere else right now -- as far as I know, no other model changes the classification head's configuration beyond default, so the default is always picked up... it's actually not read dynamically from the config currently (I think), just applied from the default. But the TimmPresetLoader solution will work for Timm models and probably this PR, but I think currently there's no clear way to change image classification heads at least from the config -- but maybe there is a different path for non-Timm models?

I think updating the base class won't solve the issue as every classifier head is a little different -- we seem to be handling this fine via subclassing though there is a weird inheritance pattern we have (calling Task.init directly, as far as I can tell it's because how we define the functional model and the order of calling the super constructors).

[pkgoogle]

So the current CI will still fail for the same reason as before, but I did make this update.

Current issue:

_________________________________________________________________ MobileNetImageClassifierTest.test_all_presets __________________________________________________________________

self = <keras_hub.src.models.mobilenet.mobilenet_image_classifier_test.MobileNetImageClassifierTest testMethod=test_all_presets>

    @pytest.mark.large
    def test_all_presets(self):
        for preset in MobileNetImageClassifier.presets:
>           self.run_preset_test(
                cls=MobileNetImageClassifier,
                preset=preset,
                input_data=self.images,
                expected_output_shape=(2, 1000),
            )

keras_hub/src/models/mobilenet/mobilenet_image_classifier_test.py:90: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
keras_hub/src/tests/test_case.py:647: in run_preset_test
    instance = cls.from_preset(preset, **init_kwargs)
keras_hub/src/models/task.py:198: in from_preset
    return loader.load_task(cls, load_weights, load_task_weights, **kwargs)
keras_hub/src/utils/preset_utils.py:696: in load_task
    task.load_task_weights(task_weights)
keras_hub/src/models/task.py:208: in load_task_weights
    keras.saving.load_weights(
../../miniforge3/envs/keras_migration/lib/python3.10/site-packages/keras/src/saving/saving_api.py:251: in load_weights
    saving_lib.load_weights_only(
../../miniforge3/envs/keras_migration/lib/python3.10/site-packages/keras/src/saving/saving_lib.py:611: in load_weights_only
    _raise_loading_failure(error_msgs, warn_only=skip_mismatch)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

error_msgs = {6045933600: (<Dense name=predictions, built=True>, ValueError("Layer 'predictions' expected 2 variables, but received...Layer 'classifier_conv' expected 2 variables, but received 0 variables during loading. Expected: ['kernel', 'bias']"))}
warn_only = False

    def _raise_loading_failure(error_msgs, warn_only=False):
        first_key = list(error_msgs.keys())[0]
        ex_saveable, ex_error = error_msgs[first_key]
        msg = (
            f"A total of {len(error_msgs)} objects could not "
            "be loaded. Example error message for "
            f"object {ex_saveable}:\n\n"
            f"{ex_error}\n\n"
            "List of objects that could not be loaded:\n"
            f"{[x[0] for x in error_msgs.values()]}"
        )
        if warn_only:
            warnings.warn(msg)
        else:
>           raise ValueError(msg)
E           ValueError: A total of 2 objects could not be loaded. Example error message for object <Conv2D name=classifier_conv, built=True>:
E           
E           Layer 'classifier_conv' expected 2 variables, but received 0 variables during loading. Expected: ['kernel', 'bias']
E           
E           List of objects that could not be loaded:
E           [<Conv2D name=classifier_conv, built=True>, <Dense name=predictions, built=True>]

It seems like Keras doesn't save the task weights correctly and it fails to find the right weights on loading... any idea on how to dig into this? I also want to note.. this only happens for the "large" presets, the "small" ones still work fine.

[mattdangerw]

I'm not sure, but none of the saved task.json's have a num_features key. So what I'd guess...

• We have not resaved the entire classifier with these changes and uploaded. classifier.save_to_preset(path).
• Because the earlier versions of this model did not have a num_features key, we load a task with the default num_features=1024, it expect some post pooling conv weights, but hey are not there, because these files were saved before this compat breaking change.

But before we get too far on this. Is there a reason we don't have this same conv for say resnet 50? I see num_features: 2048 in the timm config for that model. Basically, I'm wondering if this is something we should fix for all our image classifier models, in which case we should add support for this into the base ImageClassifier. If not, we can add special cased to mobilenet.

[pkgoogle]

Hmm... I'm not sure but I can answer that question at least. Digging into the timm implementation, https://github.com/huggingface/pytorch-image-models/blob/main/timm/layers/classifier.py#L47 This is how they define the classifier head that Resnet uses. num_features is usually the input features to the pooling layer. This is implicit in Keras. For the MobileNet Classifier it's used multiple times: https://github.com/huggingface/pytorch-image-models/blob/main/timm/models/mobilenetv3.py#L120, the way I'm currently using it best matches num_pooled_chs = self.num_features * self.global_pool.feat_mult(). The feature multiplier is usually just 1: https://github.com/huggingface/pytorch-image-models/blob/main/timm/layers/adaptive_avgmax_pool.py#L23. So one possible solution.. is to just grab it from the backbone instead of the config... that would match the rest of the implementations better -- though I do sort of feel like there should be some way to configure the head from the config.

[pkgoogle]

Actually, I can't get it from the backbone -- Mobilenetv3 actually uses num_features differently, self.head_hidden_size = num_features # features of conv_head, pre_logits output The hidden size is grabbed from the argument (which matches the config), not the redefined class variable self.num_features, which is indeed the input channels for the pooling layer. So I think I will see if I can solve the the saving/loading issue for now.

[pkgoogle]

Yup, the smaller issue was an outdated task.json, all presets are working at least locally now. Thanks!

[mattdangerw]

Sounds good! Let's go for this. Does look like timm allows the final projection to be a conv instead of a dense, we could consider adding something like that at some point. But since mobilenet is pool -> conv -> dense that seems worth keeping as it's own subclass. Thanks!

[mattdangerw]

Oops actually sorry, one more issue. This will forward num_features to all classes, even for classifiers that don't support the kwarg. Maybe switch this to if "num_features" in self.config and "mobilenet" in self.config["architecture"]:

[pkgoogle]

done