Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add support for all models #884

Merged
merged 1 commit into from
Jun 26, 2021
Merged

Add support for all models #884

merged 1 commit into from
Jun 26, 2021

Conversation

DocGarbanzo
Copy link
Contributor

Add back support for all models

Overview

  • Factor out model from interpreter so each model can be saved and run in .h5 / .savedmodel keras format, in tflite format or in tensorrt format
  • Improved KerasPilot interfaces to support the above
  • Added test for all models and all interpreters (TFflite excluded for 3d CNN and TRT excluded for LSTM as these are not supported respectively)

UI improvements

  • Add support for all model / interpreter combinations in training and model arena
  • Add options to sync only h5, savedmodel, tflite or tensorrt pilots to car

Small improvements / fixes

  • Added comment to status in train screen
  • Added better error message in trt conversion
  • Added seq length to lstm model str
  • Added shebang to profile.py
  • In UI: fixed disabling of keyboard when switching from one screen to another.

Change on models

  • Convert LSTM and 3D-CNN model from Sequential to Functional form, because transfer learning, i.e. continuing to train an already trained model, only works in the functional form of these two models.
  • Added new model KerasLinearMemory which reads in the last n actions, this facilitates smoother steering
  • Added support to inference from dictionary - passing by dictionary is used in training and supports augmentation
  • Renamed output layers alphabetically in order to avoid tflite bug which returns output tensors in that order

General code changes

  • Add Sequencer class which returns list of continuous records so sequence models, like rnn, 3dcnn, memory can be trained. Add test.
  • Add comparison string (called CString) class to better manage printing available user options. Used in get_model_by_type.
  • Reduced tolerance on keras test as we still see relative differences of 2e-5 (which is ok).
  • Check either relative or absolute tolerance as untrained models might return values >> 1 in keras test
  • Added config parameters CREATE_TF_LITE, CREATE_TENSOR_RT to switch automatic creation of tflite and tensorrt models.

tikurahul
tikurahul reviewed Jun 21, 2021
View reviewed changes
donkeycar/management/graph.py Outdated Show resolved Hide resolved
donkeycar/parts/interpreter.py Outdated Show resolved Hide resolved
donkeycar/pipeline/types.py Outdated Show resolved Hide resolved
donkeycar/pipeline/types.py Outdated Show resolved Hide resolved
donkeycar/utils.py Outdated Show resolved Hide resolved
donkeycar/parts/interpreter.py Show resolved Hide resolved
donkeycar/parts/interpreter.py Outdated Show resolved Hide resolved
donkeycar/parts/interpreter.py Outdated Show resolved Hide resolved
donkeycar/pipeline/types.py Show resolved Hide resolved
@tikurahul
Copy link
Collaborator

Also wanted to say, this is really amazing work !

Thanks for doing this.

@DocGarbanzo
Copy link
Contributor Author

@tikurahul - I've addressed all your points above and pushed a 3rd commit with these changes for review. I can rebase and squash everything afterwards (or squash merge the 3 commits), let me know your prefs.

tikurahul
tikurahul approved these changes Jun 25, 2021
View reviewed changes
donkeycar/utils.py Outdated
@@ -30,7 +30,7 @@
ONE_BYTE_SCALE = 1.0 / 255.0


class CString:
class ComparisonString:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Call this EqMemorizedString perhaps.

@DocGarbanzo
Add support for all other (except KerasLatent) models:
21f236d 
* Factor out model from interpreter so each model can be saved and run in .h5 / .savedmodel keras format, in tflite format or in tensorrt format
* Improved KerasPilot interfaces to support the above
* Added test for all models and all interpreters (TFflite excluded for 3d CNN and TRT excluded for LSTM as these are not supported respectively)
* use expand_dims and squeeze instead of reshape and [0]

UI improvements:
* Add support for all model / interpreter combinations in training and model arena
* Add options to sync only h5, savedmodel, tflite or tensorrt pilots to car
* Fixed disabling of keyboard when switching from one screen to another

Small improvements / fixes:
* Added comment to status in train screen
* Added better error message in trt conversion
* Added seq length to lstm model str
* Added shebang to profile.py

Change on models:
* Convert LSTM and 3D-CNN model from Sequential to Functional form, because transfer learning, i.e. continuing to train an already trained model, only works in the functional form of these two models.
* Added new model KerasLinearMemory which reads in the last n actions, this facilitates smoother steering
* Added support to inference from dictionary - passing by dictionary is used in training and supports augmentation
* Renamed output layers alphabetically in order to avoid tflite bug which returns output tensors in that order

General code changes:
* Add Collator class which returns list of continuous records so sequence models, like rnn, 3dcnn, memory can be trained. Add test.
* Add comparison string class (called EqMemorizedString) to better manage printing available user options. Used in get_model_by_type.
* Reduced tolerance on keras test as we still see relative differences of 2e-5 (which is ok).
* Check either relative or absolute tolerance as untrained models might return values >> 1 in keras test
@DocGarbanzo DocGarbanzo force-pushed the extend_tflite_support_squashed branch from 40f2fc9 to 21f236d Compare June 26, 2021 13:36
@DocGarbanzo DocGarbanzo merged commit b544241 into autorope:dev Jun 26, 2021
BillyCheung10botics pushed a commit to robocarstore/donkeycar that referenced this pull request Jul 13, 2021
fix bugs and improve the approach making frames
0e9f28d 
1.  fix the bugs originated from the latest commit b544241 (Add support for all other (except KerasLatent) models: (autorope#884))

2.  improve the approach for calculating saliency maps,
2.1 evaluate the effects of all output layers instead of one
2.2 correct the problematic math (blending the gradients of angle and throttle output layers together) from direct sum to square norm
2.3 calculate the saliency depending on the model type "linear" and "categorical"

3.  modify the "draw_user_input", "draw_model_prediction", and "draw_steering_distribution" methods, seperate the image array for inference and the image array that labels are drawn on
@fengye fengye mentioned this pull request Jul 17, 2021
Closed
sctse999 pushed a commit that referenced this pull request Jul 21, 2021
@BillyCheung10botics
fix bugs and improve the approach making frames (#900)
5e2c560 
1.  fix the bugs originated from the latest commit b544241 (Add support for all other (except KerasLatent) models: (#884))

2.  improve the approach for calculating saliency maps,
2.1 evaluate the effects of all output layers instead of one
2.2 correct the problematic math (blending the gradients of angle and throttle output layers together) from direct sum to square norm
2.3 calculate the saliency depending on the model type "linear" and "categorical"

3.  modify the "draw_user_input", "draw_model_prediction", and "draw_steering_distribution" methods, seperate the image array for inference and the image array that labels are drawn on
@DocGarbanzo DocGarbanzo deleted the extend_tflite_support_squashed branch November 29, 2022 22:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tikurahul tikurahul tikurahul approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@DocGarbanzo @tikurahul