adding resnet18 support #694

cl3m3nt · 2020-12-10T09:33:09Z

Adding pre-trained Resnet18 model for transfer learning.
Model has been trained using CIFAR100 coarse (20 super classes).

Changes are the following:

adding keras_resnet18.py part (a full "manual implementation of resnet18 as it's not available from keras applications)
updating keras.py part to load keras_resnet18 part & define Resnet18LinearKeras and Resnet18CategoricalKeras classes
updating tflite.py part to optionally take into account inference on TPU Edge
updating utils.py to take into account new model type (resnet18)
updating train.py to take into account new model type (resnet18)

DocGarbanzo

Hi @cl3m3nt,
Thanks for sending this over. It looks good partially but some bits missing:

You don't seem to have the newest version of dev - can you please re-base your branch?
Do you have an IDE which shows pep-8 compatibility (like PyCharm etc). There are a lot of non-compliant format issues, which these IDEs will automatically fix for you.
There is an import error in the CI, you should have that locally too, which version of TF do you use locally?
Can you please add a test into test_keras.py that shows the model is working?

DocGarbanzo · 2020-12-18T19:45:18Z

donkeycar/parts/keras.py

+
+def resnet18_default_n_linear(num_outputs, input_shape=(120,60,3)):
+
+ # Instantiate a ResNet18 model


Instead of a comment, please rather provide a doctoring.

What is a doctoring :) ?

That's a typo (autocomplete :-) meant to be 'docstring'.

DocGarbanzo · 2020-12-18T19:46:12Z

donkeycar/parts/keras.py

 from tensorflow.keras.layers import Activation, Dropout, Flatten
 from tensorflow.keras.layers import LSTM
 from tensorflow.keras.layers import TimeDistributed as TD
 from tensorflow.keras.layers import Conv3D, MaxPooling3D, Conv2DTranspose
 from tensorflow.keras.backend import concatenate
 from tensorflow.keras.models import Model, Sequential
+from donkeycar.parts.keras_resnet18 import identity_block,conv_block,ResNet18


This line falls over in the CI, please also use pep-8 formatting

Any guess why it falls over in CI ?
Might be related to keras_resnet18 import
'from keras_applications.imagenet_utils' ?

DocGarbanzo · 2020-12-18T19:47:34Z

donkeycar/parts/keras.py

+utils = tf.keras.utils
+
+
+def resnet18_default_n_linear(num_outputs, input_shape=(120,60,3)):


As you already made a new module for the new model, pls put those functions there.

What do you mean here ? Do you recommend to move resnet18 function and classes to keras_resnet18 module ?

DocGarbanzo · 2020-12-18T19:48:05Z

donkeycar/parts/keras.py

+def resnet18_default_n_linear(num_outputs, input_shape=(120,60,3)):
+
+ # Instantiate a ResNet18 model
+ resnet18 = ResNet18(include_top=False,weights='cifar100_coarse',input_shape=input_shape,backend=backend,layers=layers,models=models,utils=utils)


Please use pep-8 line length and formatting.

DocGarbanzo · 2020-12-18T19:49:38Z

donkeycar/parts/keras.py

+ img_arr = img_arr.reshape((1,) + img_arr.shape)
+ outputs = self.model.predict(img_arr)
+ steering = outputs[0]
+ return steering[0] , dk.utils.throttle(steering[0])


This is the behaviour of KerasInferred, KerasLinear returns steering and throttle independently.

Yes you are right, I'll fix this following KerasLinear.

DocGarbanzo · 2020-12-18T19:55:03Z

donkeycar/parts/keras_resnet18.py

+ name = conv_name_base + '2b')(x)
+ x = BatchNormalization(axis=bn_axis,name=bn_name_base+'2b')(x)
+ x = Activation('relu')(x)
+


DocGarbanzo · 2020-12-18T19:56:39Z

donkeycar/parts/keras_resnet18.py

+
+
+ x = Convolution2D(filters2, kernel_size,
+ padding='same',


Indention: pep-8.

DocGarbanzo · 2020-12-18T19:57:53Z

donkeycar/parts/keras_resnet18.py

+ pooling=None,
+ classes=20,
+ **kwargs):
+ global backend, layers, models, keras_utils


Please try to avoid global variables. If yo need to share them between functions it's fine to put them into a class.

I followed here what keras_team did for resnet50
https://github.com/keras-team/keras-applications/blob/master/keras_applications/resnet50.py

DocGarbanzo · 2020-12-18T20:00:13Z

donkeycar/parts/keras_resnet18.py

+
+def identity_block(input_tensor, kernel_size, filters, stage, block):
+ filters1, filters2 = filters
+ if backend.image_data_format() == 'channels_last':


This following code seems to re-appear a couple of times, maybe factor this into a small helper function.

Yes, same here I followed what keras-team did for resnet50.
There a 2 occurences of the same code, which could be acceptable don't you think ?

DocGarbanzo · 2020-12-18T20:06:38Z

donkeycar/templates/train.py

@@ -4,136 +4,117 @@
 Basic usage should feel familiar: python train_v2.py --model models/mypilot

 Usage:
- train.py [--tubs=tubs] (--model=<model>) [--type=(linear|inferred|tensorrt_linear|tflite_linear)]
+ train.py [--tubs=tubs] (--model=<model>) [--type=(linear|inferred|tensorrt_linear|tflite_linear|tflite_r18_lin|tflite_r18_cat)]

 Options:
 -h --help Show this screen.
 """



You don't have the HEAD revision of the dev branch, this file is now only a small script supported for backward compatibility.

Ok I'll double check then how you train within the HEAD revision.

DocGarbanzo · 2021-01-16T14:08:59Z

Hi @cl3m3nt - do you still want to pursue this? We have a reset 18 in pytorch now, but there is no harm having this in tf, too?

sctse999 · 2021-01-27T04:54:26Z

@cl3m3nt this looks great and I believe a lot of people would like to see this merged. Do you have time to work on this?

tikurahul · 2021-01-27T05:23:19Z

We already have Resnet18 with PyTorch now. FYI

cl3m3nt · 2021-01-27T17:48:37Z

Thanks @sctse999.
Yes I could find some extra time to update it so that it meets @DocGarbanzo guidance.
But I would also need to adapt taking into account last dev significant updates. I need to evaluate the effort before. Also I was considering making a PR to keras team so that Resnet18 would be available natively out from tf.keras.applications. Let me take some time to rethink about that.

cl3m3nt · 2021-01-27T17:53:01Z

We already have Resnet18 with PyTorch now. FYI

Thanks @tikurahul for the heads up.
I'll figure out whether I can update anyway this PR so that Resnet18 is available from keras as well.
Meanwhile, do you think that mobilenetv2 would make sense to be available ? Resnet18 got 7 Millions parameters vs Mobilenet which comes 2.2 Millions only. Might be interesting to have it as well to decrease latency when using Transfer Learning. What do you think ?

sctse999 · 2021-01-28T03:16:14Z

Great I would love to see resnet18 or mobilenetv2 on keras as well.

adding resnet18 support

bd482f6

DocGarbanzo requested changes Dec 18, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

adding resnet18 support #694

adding resnet18 support #694

cl3m3nt commented Dec 10, 2020

DocGarbanzo left a comment

DocGarbanzo Dec 18, 2020

cl3m3nt Dec 22, 2020

DocGarbanzo Jan 16, 2021

DocGarbanzo Dec 18, 2020

cl3m3nt Dec 22, 2020

DocGarbanzo Dec 18, 2020

cl3m3nt Dec 22, 2020

DocGarbanzo Dec 18, 2020

cl3m3nt Dec 22, 2020

DocGarbanzo Dec 18, 2020

cl3m3nt Dec 22, 2020

DocGarbanzo Dec 18, 2020

cl3m3nt Dec 22, 2020

DocGarbanzo Dec 18, 2020

cl3m3nt Dec 22, 2020

DocGarbanzo Dec 18, 2020

cl3m3nt Dec 22, 2020

DocGarbanzo Dec 18, 2020

cl3m3nt Dec 22, 2020

DocGarbanzo Dec 18, 2020

cl3m3nt Dec 22, 2020

DocGarbanzo commented Jan 16, 2021

sctse999 commented Jan 27, 2021

tikurahul commented Jan 27, 2021

cl3m3nt commented Jan 27, 2021

cl3m3nt commented Jan 27, 2021

sctse999 commented Jan 28, 2021


		def resnet18_default_n_linear(num_outputs, input_shape=(120,60,3)):

		# Instantiate a ResNet18 model

		utils = tf.keras.utils


		def resnet18_default_n_linear(num_outputs, input_shape=(120,60,3)):

adding resnet18 support #694

Are you sure you want to change the base?

adding resnet18 support #694

Conversation

cl3m3nt commented Dec 10, 2020

DocGarbanzo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DocGarbanzo commented Jan 16, 2021

sctse999 commented Jan 27, 2021

tikurahul commented Jan 27, 2021

cl3m3nt commented Jan 27, 2021

cl3m3nt commented Jan 27, 2021

sctse999 commented Jan 28, 2021