mocleiri / tensorflow-micropython-examples

A custom micropython firmware integrating tensorflow lite for microcontrollers and ulab to implement the tensorflow micro examples.
MIT License
183 stars 87 forks source link

Enabled support for ESP-NN optimizations #120

Closed vikramdattu closed 1 year ago

vikramdattu commented 1 year ago

Following changes were done:

  1. Duplicated openmv-libtf.cpp to openmv-libtf-updated.cpp and used python_ops_resolver instead of all_ops_resolver which has been removed from latest tflite-micro sources.
  2. Added python_ops_resolver sources from tflite-micro
  3. Declutter: Create separate .cmake for esp specfic boards
  4. Added ESP-NN sources to micropython.cmake and micropython_esp.cmake
vikramdattu commented 1 year ago

@mocleiri this needs a small fix in ESP-NN, after which this can be merged. (Marked it draft till then) You may go ahead and review this still.

mocleiri commented 1 year ago

@vikramdattu thanks for this PR. I will merge it when you tell me the ESP-NN fixes are in.

vikramdattu commented 1 year ago

@vikramdattu thanks for this PR. I will merge it when you tell me the ESP-NN fixes are in.

Sure, will do. Initially, I thought it would be straight forward, but since, the tflite-micro commit you are using seems quite some old.

I am thinking of using all the files from tflite-micro-esp-examples but that would need me to do #ifdefs in some files (eg., openmv-libtf.cpp) as some of the headers and tflite APIs used here are not valid anymore with latest tflite tip.

@mocleiri would you be updating tflite-micro used here to the latest? This would make it cleaner and less platform specific code changes in common files when I add support for esp-nn.

vikramdattu commented 1 year ago

@mocleiri update: I have now enabled ESP-NN successfully. 🥳 Since, I found some bugs in esp-nn, fix for which is underway, I will hold the PR until it is fixed, update esp-tflm source submodule. Then this can be merged.

mocleiri commented 1 year ago

@vikramdattu thanks for the update and for getting it working.

MATTYGILO commented 1 year ago

@vikramdattu This would be awesome if you could get it done!!!

vikramdattu commented 1 year ago

@mocleiri @MATTYGILO this is done except...

The compile options set in the micropython.cmake are not actually getting set to source files. Specifically, this file needs to not include a header depending on TF_LITE_STATIC_MEMORY flag!

I have checked all the options and not able to figure out the issue. :(

If I set this compile option directly to global file micropython/ports/esp32/main/CMakeList.txt, the build is successful. But that's not the correct way of course!

Probably someone has already figured out the issue. @mocleiri Do other platforms get these compile options in source files?

mocleiri commented 1 year ago

@vikramdattu I've been setting the TF_LITE_STATIC_MEMORY flag directly its not being included from any header file.

I'm setting it here for esp32: https://github.com/mocleiri/tensorflow-micropython-examples/blob/main/micropython-modules/microlite/micropython.cmake#L370

I remember scraping the option from what I saw building tflm separately and then just had the option included within the micropython build.

Here it is for RP2: https://github.com/mocleiri/tensorflow-micropython-examples/blob/main/micropython-modules/microlite/micropython.cmake#L332

Here it is for the Gnu Male builds: https://github.com/mocleiri/tensorflow-micropython-examples/blob/main/micropython-modules/microlite/micropython.cmake#L332

Tomorrow I'll take a look at your branch to see if I can figure anything out.

vikramdattu commented 1 year ago

@mocleiri thanks for the reply. I have identified a potential issue with the micro python build system, which does a separate pre-processing step for itself that does not include compile options.

Looking at the ports/esp32/main/CMakeList.txt, I noticed following code snippet:

# Collect all of the include directories and compile definitions for the IDF components.
foreach(comp ${IDF_COMPONENTS})
    micropy_gather_target_properties(__idf_${comp})
endforeach()

Based on this information, it appears that the pre-processing step does not take into account the flags passed to the microlite component. Although the build step itself is successful, the issue arises specifically during the pre-processing stage.

To address this problem, I made an adjustment by adding the micropy_gather_target_properties(microlite) call in the microsite/micropython.cmake file. This modification successfully resolves the error. Please note that including micropython/py/py.cmake is necessary for the aforementioned macro to function correctly.

Ideally, if you update the sources, the issue would pop up for all the targets and by incorporating the above line, this issue should be resolved for all other targets as well.

Please check the top commit for the change: https://github.com/vikramdattu/tensorflow-micropython-examples/commit/127ff3d183f019f5802a1e02c6f22be072b2addc

mocleiri commented 1 year ago

I have my development environment restored and I took a look at your branch.

I'm working my way to understand the exact issue you are facing.

I was able to integrate the esp-camera idf-component into the microlite firmware for the MICROLITE_SPIRAM_CAM board.

I was able to get add the ESP-NN in previously using the approach here: https://github.com/micropython/micropython/issues/8041#issuecomment-1013624950

In the board CMakeLists.txt file you specify the directory of the extra idf-components: set(EXTRA_COMPONENT_DIRS ../../../tflm_esp_kernels/components/esp-nn)

Then in the microlite_esp.cmake you list the idf components to include: set (COMPONENTS esp-nn)

I'm trying the same approach on the latest micropython to see if it still works and if it can be used to bring in the tflite-lib directly as an idf-component.

This way the idf-component configuration for the tflite-lib should still work as you coded and then it just needs to link with the microlite code to add the micropython bindings.

I don't have it building yet as I need to add some additional include directories.

https://github.com/mocleiri/tensorflow-micropython-examples/tree/experiment-using-microlite-on-tflite-lib

vikramdattu commented 1 year ago

@mocleiri if you checkout my branch, it is already working and you can check it for yourself.

Your way for sure, looks cleaner way to do things. Looking forward to your final implementation. Please do update the esp_tflm submodule as it has the latest changes and fixes to esp-nn. Please don't be shy cherry-picking changes from my branch if that helps. (You will especially need a fix to include micro_op_resover with latest sources).

MATTYGILO commented 1 year ago

@vikramdattu Was trying to implement esp_nn with the newest micropython and have been getting this error. How do I fix it?

CMakeFiles/micropython.elf.dir/tflite-micro-esp-examples/components/esp-nn/src/convolution/esp_nn_depthwise_conv_s8_mult1_3x3_padded_esp32s3.S.obj: in function `esp_nn_depthwise_conv_s8_mult1_3x3_padded_esp32s3':
(.text+0x346): dangerous relocation: call8: call target out of range: esp_nn_multiply_by_quantized_mult_ver1_esp32s3
(.text+0x355): dangerous relocation: call8: call target out of range: esp_nn_multiply_by_quantized_mult_ver1_esp32s3
(.text+0x364): dangerous relocation: call8: call target out of range: esp_nn_multiply_by_quantized_mult_ver1_esp32s3
(.text+0x373): dangerous relocation: call8: call target out of range: esp_nn_multiply_by_quantized_mult_ver1_esp32s3
CMakeFiles/micropython.elf.dir/tflite-micro-esp-examples/components/esp-nn/src/convolution/esp_nn_depthwise_conv_s16_mult1_3x3_esp32s3.S.obj: in function `esp_nn_depthwise_conv_s16_mult1_3x3_esp32s3':
(.text+0x20d): dangerous relocation: call8: call target out of range: esp_nn_multiply_by_quantized_mult_ver1_esp32s3
(.text+0x225): dangerous relocation: call8: call target out of range: esp_nn_multiply_by_quantized_mult_ver1_esp32s3
CMakeFiles/micropython.elf.dir/tflite-micro-esp-examples/components/esp-nn/src/convolution/esp_nn_depthwise_conv_s16_mult8_3x3_esp32s3.S.obj: in function `esp_nn_depthwise_conv_s16_mult8_3x3_esp32s3':
(.text+0x1cf): dangerous relocation: call8: call target out of range: esp_nn_multiply_by_quantized_mult_ver1_esp32s3
(.text+0x1e6): dangerous relocation: call8: call target out of range: esp_nn_multiply_by_quantized_mult_ver1_esp32s3
CMakeFiles/micropython.elf.dir/tflite-micro-esp-examples/components/esp-nn/src/convolution/esp_nn_depthwise_conv_s16_mult4_esp32s3.S.obj: in function `esp_nn_depthwise_conv_s16_mult4_esp32s3':
(.text+0x1f2): dangerous relocation: call8: call target out of range: esp_nn_multiply_by_quantized_mult_ver1_esp32s3
CMakeFiles/micropython.elf.dir/tflite-micro-esp-examples/components/esp-nn/src/convolution/esp_nn_depthwise_conv_s16_mult8_esp32s3.S.obj: in function `esp_nn_depthwise_conv_s16_mult8_esp32s3':
(.text+0x1f1): dangerous relocation: call8: call target out of range: esp_nn_multiply_by_quantized_mult_ver1_esp32s3
(.text+0x209): dangerous relocation: call8: call target out of range: esp_nn_multiply_by_quantized_mult_ver1_esp32s3

Note I have added these:

CONFIG_IDF_TARGET="esp32s3"
CONFIG_IDF_TARGET_ESP32S3=y
mocleiri commented 1 year ago

@vikramdattu you're right. Lets integrate your work as written and get the firmware and flash some boards and verify its working right for the reference examples. I can carry a micropython fork in the short term for your upstream change.

Then work over time to see if tieing in at the idf-component level can get around the micropython upstream change. I can file a seperate ticket for this.

vikramdattu commented 1 year ago

@MATTYGILO please checkout my branch from this PR and use micropython version used by this project. It should work as is. I have not tried with lated micro python. But by looking at your error, you need to pass -mlongcalls flag to get around the issue.