Makes MobileNetV3 a Functional subclass model & removes closures #1475
Conversation
ID6109
changed the title
…MobileNetV3 � Conflicts: � keras_cv/models/mobilenet_v3.py
keras_cv/models/mobilenet_v3.py
Outdated
| @@ -112,48 +112,44 @@ def depth(x, divisor=8, min_value=None): | |||
| return new_x | |||
|
|
|||
|
|
|||
| def HardSigmoid(name=None): | |||
| def HardSigmoid(x, name=None): | |||
There was a problem hiding this comment.
let's rename these activation functions (and the InvertedResBlock) to use the apply_xyz convention
(e.g. apply_hard_sigmoid, apply_inverted_res_block)
keras_cv/models/mobilenet_v3.py
Outdated
| """Instantiates the MobileNetV3 architecture. | ||
|
|
||
| References: | ||
| - [Searching for MobileNetV3](https://arxiv.org/pdf/1905.02244.pdf) (ICCV 2019) | ||
| - [Based on the Original keras.applications MobileNetv3](https://github.com/keras-team/keras/blob/master/keras/applications/mobilenet_v3.py) | ||
|
|
||
| This function returns a Keras MobileNetV3 model. | ||
| This class returns a Keras MobileNetV3 model. |
There was a problem hiding this comment.
While you're editing this, would you mind also adding type indications to these docstrings?
(See, for example, docstring updates to VGG19 in #1471)
|
The changes have been made. I'll implement the same in my other PRs. Do let me know if there are further corrections needed. Thanks. |
keras_cv/models/mobilenet_v3.py
Outdated
|
|
||
| Returns: | ||
| A `keras.Model` instance. | ||
| """ | ||
|
|
||
|
|
||
| def depth(x, divisor=8, min_value=None): | ||
| """Ensure that all layers have a channel number that is divisble by the `divisor`. | ||
| def apply_depth(x, divisor=8, min_value=None): |
There was a problem hiding this comment.
nit: this isn't a function on a Tensor, but a helper method to get the number of channels for a convolutional layer, so it should just be depth instead of apply_depth
keras_cv/models/mobilenet_v3.py
Outdated
|
|
||
| Args: | ||
| x: input value. | ||
| divisor: integer, the value by which a channel number should be divisble, | ||
| x: input tensor. |
There was a problem hiding this comment.
This isn't expected to be a tensor -- it should be an integer scalar
There was a problem hiding this comment.
It seems I skimmed past this method in a hurry. I'll be more mindful in the future. Thanks.
There was a problem hiding this comment.
No worries at all -- thank you for the contributions!
| name: string, layer label. | ||
|
|
||
| Returns: | ||
| a function that takes an input Tensor representing a HardSigmoid layer. | ||
| the updated input tensor. |
There was a problem hiding this comment.
(throughout) - it would be more accurate to say that this returns "the result of applying hard sigmoid to the input tensor"
There was a problem hiding this comment.
Actually, this looks consistent with the docs in other models, so feel free to ignore this for now and we can address it in all the models at once in #1500
There was a problem hiding this comment.
I'll take this up directly while solving #1500 then.
|
Updated |
|
/gcbrun |
…as-team#1475) * Generates Error * Reformat * num_classes change * reformatted * Final * Brush * Brush * Changed function names and added type indications * Minor changes
What does this PR do?
Fixes #1427
Colab
Who can review?
@ianstenbit @LukeWood @tanzhenyu