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.

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

module update #6

Closed
wants to merge 260 commits into from
Closed

Conversation

JordanYates
Copy link

Update tflite-micro to a version compatible with CMSIS-NN v6.0.0

turbotoribio and others added 30 commits July 13, 2023 19:27
`port c++ energy op to open source in tflm_signal`

-port energy op and corresponding to new open source location for C++

BUG=[b/289422411](https://b.corp.google.com/issues/289422411)
`port c++ filter_bank ops to open source in tflm_signal`

-port filter_bank ops and corresponding to new open source location for C++

BUG=[b/289422411](https://b.corp.google.com/issues/289422411)
This will (hopefully) ensure that RISC-V failures result in a github issue being created.

BUG=http://b/291335234
The generic benchmarking utilities added in PR 2125 did not compile on RISC-V due to two separate issues:

 * An incorrect format specifier for int32_t on RISC-V. This is resolved by using PRId32 instead of %ld or %d.
 * The fileno() is undeclared with the RISC-V toolchain, similar to how it is with the Embedded ARM toolchain. This PR resolves this by forgoing the fstat() check on file size (as this is what used fileno), and instead attempting to read the maximum number of bytes we can fit in the model buffer. We can then use feof() to verify that we read the entirety of the model, and error out if we did not. This solution also eliminates needing to worry about different file types, such as how the Xtensa simulator treats files from the host system as character devices.

BUG=2129
This PR adds additional FFT op functionality in the Signal library, namely adding the FFT Auto Scale operation.
Testing added in the original `fft_test.cc` and `fft_ops_test.py`.
BUG=[287346710](http://b/287346710)
BUG=[549343727](http://cl/549343727)
Extends the Signal Library Delay OP to be usable from python.
Can test via `bazel run python/tflite_micro/signal:delay_op_test`

BUG=[287346710](http://b/287346710)
Inverse-RFFT as part of Signal library ops.
Testing via current FFT Op tests.

BUG=[287346710](http://b/287346710)
Our Cortex-m55 build is failing for fft_test.cc due to strict tolerance in irfft tests. Increasing tolerance slightly similar to what we did for rfft.

Can test locally with
```make -f tensorflow/lite/micro/tools/make/Makefile -j24 test_kernel_signal_fft_test TARGET=cortex_m_corstone_300 TARGET_ARCH=cortex-m55```

BUG=[287518815](http://b/287518815)
…low#2142)

We this PR, you can use these ops directly from python, including in TF graphs.

Test with `bazel run python/tflite_micro/signal:framer_op_test`, etc.

BUG=[287346710](http://b/287346710)
…#2143)

Functionality to use the rest of the Signal Library OPs directly from python.

Test with `bazel run python/tflite_micro/signal:stacker_op_test` and `bazel run python/tflite_micro/signal:filter_bank_ops_test`

BUG=[287346710](http://b/287346710)
With tensorflow#1818, TFLM does not need the TfLite OpResolver.

BUG=http://b/272808609
…nsorflow#2151)

Add the build configuration and integrated test to generate a Python
distribution package named `tflite_micro` for publishing the tflm interpreter
as a Python module with a native extension.

Use the build tools provided in @rules_python, augmented by a custom rule
`py_namespace` for the reasons documented in `python/py_namespace.bzl`.

Provide an integration test at `//python/tflite_micro:whl_test`. Use a .tflite
model copied from the hello_world example. (Copied to avoid creating a
dependency.)

BUG=part of tensorflow#1484
Adds support for 16-bit activations + 8-bit weights for depthwise convolution in the reference kernel. Uses 64-bit bias to match TFLite. Also adds passthrough to the q16x8 reference kernel for Xtensa, CEVA, and ARC (CMSIS already has it's own implementation).

Tested:
depthwise_conv_test

BUG=2141
Upgrade @rules_python in preparation for adding build stamp information to the
tflite_micro Python distribution package. This new version contains necessary
fixes.

Move the call to workspace(), which defines several external repositories,
ahead of the repository definitions in WORKSPACE itself. workspace() defines
@bazel_skylib, which is needed by the new version of @rules_python, and
therefore must be defined first.

Add a few comments about the use of @rules_python.

BUG=part of tensorflow#1484
BUG=automated sync from upstream
NO_CHECK_TFLITE_FILES=automated sync from upstream
…aze/Bazel (tensorflow#2160)

`load()`s are being added in preparation for changes being made to Blaze/Bazel !

corresponding [google3 cl](https://critique.corp.google.com/cl/553849494)

BUG=[b/295216390](https://b.corp.google.com/issues/295216390)
BUG=automated sync from upstream
NO_CHECK_TFLITE_FILES=automated sync from upstream
This PR creates the initial code generator scaffolding for performing inference without an interpreter. Currently, this does nothing other create a header and source file from Mako templates. Mako was chosen as a template engine due to existing dependency.

BUG=b/295076487
This PR adds a codegen inference example for the hello world model to demonstrate how to invoke the code generator and build the generated source. For now, we're just checking the generated source into the repo to skip over building out the make rules and also ensuring the generated source complies with formatting rules.

This also fixes a minor formatting issue in the source templates, as clang-format now properly complained about it.

BUG=b/295390000
The new codegen python build rules were added without properly loading the rules.

BUG=b/295216390
* tensorflow@a76d154 has the changes needed to create test data for the current PR. I reverted the changes back to have the script be mostly unchanged (except for some typos and trailing whitespaces).
 * To get the new quantization parameters:
```bash
bazel run tensorflow/lite/micro/kernels/testdata:lstm_test_data_generator
```
And then manually copy over the parameters into `lstm_test_data.cc`

BUG=http://b/296130372
rascani and others added 6 commits May 22, 2024 17:37
When running the gen_micro_mutable_op_resolver script as a regular python script without bazel, it cannot find visualize because it is not part of the tflite-micro wheel. When running the script with bazel, bazel can pick up visualize from the path
tflite_micro/tensorflow/lite/tools as long as tflite-micro wheel is not installed. If it is installed, then bazel will only look at that, which it is not a part of.

This PR switches the import to just be from tensorflow, which should work regardless of whether it is from the tf package or relative path.

BUG=tensorflow#2564
…ow#2589)

The change introduces the support for ML models with 16 bit activations and 8 bit weights in the ExpandDims operation.

BUG=fixes #68293
The TFLiteConverter recently switched over to using per-channel quantization for all Dense/FullyConnected layers. TFLite-Micro does not yet have support for this, and was using incorrect quantization parameters for FullyConnected layers on newly converted models. Unsurprisingly, this leads to invalid output.

While we intend to add per-channel quantization support for FullyConnected, this PR adds a runtime check for per-channel quantization until it can be supported by individual kernels. If you encounter this runtime error, you can disable the new Converter behavior by setting:

`TfLiteConverter._experimental_disable_per_channel_quantization_for_dense_layers = True` https://github.com/tensorflow/tensorflow/blob/377f47694fa790e98db6665b9adecde00b5e0d68/tensorflow/lite/python/lite.py#L674

BUG=b/324385802
Add `zephyr/module.yml` to enable repo as zephyr module.

Signed-off-by: Jordan Yates <jordan@embeint.com>
Add a script which automates the process of exporting the third party
dependencies for Zephyr.

Signed-off-by: Jordan Yates <jordan@embeint.com>
Copy link

@ithinuel ithinuel left a comment

Choose a reason for hiding this comment

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

This is based on the zephyr branch rather than the main branch.

The addition of the export script seems to be a good idea to simplify future updates however it yields the wrong paths.

To review/validate this PR I ran:

git fetch pull/6/HEAD:PR6
git checkout main
git merge 71fba59b
# add & run the script
# add third_party_static
# commit
git diff PR6

mkdir -p third_party_static/kissfft

cp $DL_FOLDER/flatbuffers/LICENSE third_party_static/flatbuffers/LICENSE
cp -r $DL_FOLDER/flatbuffers/include third_party_static/flatbuffers/include

Choose a reason for hiding this comment

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

Makes it go in third_party_static/flatbuffers/include/include (duplicated include)

Suggested change
cp -r $DL_FOLDER/flatbuffers/include third_party_static/flatbuffers/include
cp -r $DL_FOLDER/flatbuffers/include third_party_static/flatbuffers

Comment on lines +19 to +20
cp -r $DL_FOLDER/gemmlowp/fixedpoint third_party_static/gemmlowp/fixedpoint
cp -r $DL_FOLDER/gemmlowp/internal third_party_static/gemmlowp/internal

Choose a reason for hiding this comment

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

Suggested change
cp -r $DL_FOLDER/gemmlowp/fixedpoint third_party_static/gemmlowp/fixedpoint
cp -r $DL_FOLDER/gemmlowp/internal third_party_static/gemmlowp/internal
cp -r $DL_FOLDER/gemmlowp/fixedpoint third_party_static/gemmlowp
cp -r $DL_FOLDER/gemmlowp/internal third_party_static/gemmlowp

cp $DL_FOLDER/kissfft/kiss_fft.c third_party_static/kissfft/kiss_fft.c
cp $DL_FOLDER/kissfft/kiss_fft.h third_party_static/kissfft/kiss_fft.h
cp $DL_FOLDER/kissfft/kissfft.hh third_party_static/kissfft/kissfft.hh
cp -r $DL_FOLDER/kissfft/tools third_party_static/kissfft/tools

Choose a reason for hiding this comment

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

Suggested change
cp -r $DL_FOLDER/kissfft/tools third_party_static/kissfft/tools
cp -r $DL_FOLDER/kissfft/tools third_party_static/kissfft

cp -r $DL_FOLDER/gemmlowp/internal third_party_static/gemmlowp/internal

cp $DL_FOLDER/ruy/LICENSE third_party_static/ruy/LICENSE
cp -r $DL_FOLDER/ruy/ruy/profiler third_party_static/ruy/ruy/profiler

Choose a reason for hiding this comment

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

Suggested change
cp -r $DL_FOLDER/ruy/ruy/profiler third_party_static/ruy/ruy/profiler
cp -r $DL_FOLDER/ruy/ruy/profiler third_party_static/ruy/ruy

@nashif
Copy link
Member

nashif commented Jun 27, 2024

ok, this is a mess, no idea what is going on here and how we got here :(

@JordanYates
Copy link
Author

This is based on the zephyr branch rather than the main branch.

The addition of the export script seems to be a good idea to simplify future updates however it yields the wrong paths.

To review/validate this PR I ran:

git fetch pull/6/HEAD:PR6
git checkout main
git merge 71fba59b
# add & run the script
# add third_party_static
# commit
git diff PR6

Paths are fine when I run it and my simple reproduction behaves as I expect, no duplicated directories:

(.zephyr_venv) jordan@TAURUS:~$ mkdir tmp
(.zephyr_venv) jordan@TAURUS:~$ cp -r ~/code/zephyr/zephyr/include/ ./tmp/include
(.zephyr_venv) jordan@TAURUS:~$ ls -ls tmp/include/
total 4
4 drwxr-xr-x 57 jordan jordan 4096 Jun 27 20:18 zephyr

@JordanYates
Copy link
Author

ok, this is a mess, no idea what is going on here and how we got here :(

I agree, a giant mess and why I would prefer a branch :)

@JordanYates
Copy link
Author

This is based on the zephyr branch rather than the main branch.

Nope, this is based purely on the upstream tensorflow micro main branch.

(.zephyr_venv) jordan@TAURUS:~/code/zephyr/optional/modules/lib/tflite-micro$ git remote -v
jordan  git@github.com:JordanYates/tflite-micro.git (fetch)
jordan  git@github.com:JordanYates/tflite-micro.git (push)
origin  git@github.com:tensorflow/tflite-micro.git (fetch)
origin  git@github.com:tensorflow/tflite-micro.git (push)
upstream        https://github.com/zephyrproject-rtos/tflite-micro (fetch)
upstream        https://github.com/zephyrproject-rtos/tflite-micro (push)
(.zephyr_venv) jordan@TAURUS:~/code/zephyr/optional/modules/lib/tflite-micro$ git log --oneline
e1eecbee (HEAD, jordan/zephyr-v3.7, zephyr-v3.7, manifest-rev, 240613_export) third_party_static: exported
73258a3e zephyr_static_export.sh: added
8be43d15 zephyr: module.yml: added
71fba59b (origin/main) Fail per-channel quantized FullyConnected layers (#2602)

Or shown a different way: https://github.com/JordanYates/tflite-micro/commits/zephyr-v3.7/

@stephanosio
Copy link
Member

ok, this is a mess, no idea what is going on here and how we got here :(

I agree, a giant mess and why I would prefer a branch :)

zephyrproject-rtos/zephyr#73995 (comment)

Let us create a new branch -- it makes sense and everyone keeps their sanity.

Now the only question is: zephyr-v3.7 vs vTFLITEMICROVER-zephyr ...

@JordanYates
Copy link
Author

Now the only question is: zephyr-v3.7 vs vTFLITEMICROVER-zephyr ...

The name is almost irrelevant since all that matters is the commit hash.
Someone with push rights make a unilateral decision and I will support it.

@stephanosio
Copy link
Member

The name is almost irrelevant since all that matters is the commit hash.

Branch names do matter because they reflect how upstream syncs are meant to be handled.

Someone with push rights make a unilateral decision and I will support it.

I say we go and stick with zephyr-v3.7 (i.e. zephyr-vZEPHYRVER) because, that way, it is easier to backport module patches for specific Zephyr release in the future -- this is especially important for an LTS too ...

@nashif Do you agree?

@ithinuel
Copy link

ithinuel commented Jun 27, 2024

This is based on the zephyr branch rather than the main branch.

Nope, this is based purely on the upstream tensorflow micro main branch.

Indeed, my bad.

Paths are fine when I run it and my simple reproduction behaves as I expect, no duplicated directories:

No they are not.

This file is in third_party_static/flatbuffers/include/include/flatbuffers/base.h and third_party_static/flatbuffers/include/flatbuffers/base.h (and all the other files and directories in flatbuffers.

You can check around third_party_static/gemmlowp/fixedpoint/fixedpoint.h as there are less files and you can see both the outer and the inner replicated directories.

Screenshot 2024-06-27 at 12 02 55

@nashif
Copy link
Member

nashif commented Jun 27, 2024

I prefer to use zephyr, when someone lands on the pages, they need to see the code being used and not some old commits.
We can tag and archive existing state and reset the 'zephyr' branch with the code.

@nashif
Copy link
Member

nashif commented Jun 27, 2024

Just give me a sha that can be used and I will do that directly.

@stephanosio
Copy link
Member

I prefer to use zephyr, when someone lands on the pages, they need to see the code being used and not some old commits. We can tag and archive existing state and reset the 'zephyr' branch with the code.

@nashif So how are we supposed to handle tflite-micro backport patches for Zephyr v3.7 when Zephyr is at v4.5 and the tflite-module zephyr branch is well into the future?

It is much easier to open/review a "backport patch PR" to zephyr-v3.7 branch than to do some behind-the-scenes chat and tagging magic.

Export third party sources with `zephyr_static_export.sh`.

Signed-off-by: Jordan Yates <jordan@embeint.com>
@JordanYates
Copy link
Author

You can check around third_party_static/gemmlowp/fixedpoint/fixedpoint.h as there are less files and you can see both the outer and the inner replicated directories.

Turns out we were both right. You were correct that files were duplicated, they were accidentally committed while testing the export script.

Reverted the entire export commit and re-ran the script, files now match the current structure.

@nashif
Copy link
Member

nashif commented Jun 27, 2024

@nashif So how are we supposed to handle tflite-micro backport patches for Zephyr v3.7 when Zephyr is at v4.5 and the tflite-module zephyr branch is well into the future?

It is much easier to open/review a "backport patch PR" to zephyr-v3.7 branch than to do some behind-the-scenes chat and tagging magic.

why are we dealing with backports and branches as if 3.7 was released? This is main branch development happening in zephyr right now, creating a 3.7 branch now and leaving 'zephyr' branch in a limbo is not really helpful. We will create a branch for 3.7 when the time comes, I do not think we need to worry about it now, this is mainstream development still.

@stephanosio
Copy link
Member

why are we dealing with backports and branches as if 3.7 was released? This is main branch development happening in zephyr right now, creating a 3.7 branch now and leaving 'zephyr' branch in a limbo is not really helpful. We will create a branch for 3.7 when the time comes, I do not think we need to worry about it now, this is mainstream development still.

Fair enough, using zephyr as the "trunk" branch and creating zephyr-vx.y release branches as they are needed in the future then, as loosely documented in https://docs.zephyrproject.org/latest/develop/modules.html#module-repositories.

@ithinuel
Copy link

Fair enough, using zephyr as the "trunk" branch and creating zephyr-vx.y release branches as they are needed in the future then, as loosely documented in docs.zephyrproject.org/latest/develop/modules.html#module-repositories.

It’s worth noting that the current zephyr branch is an old head and the current manifest-rev pulled by zephyr is main and is not related to the old zephyr.
So the old zephyr head would need to be tagged with some name or its commits may be garbage collected.

@ithinuel
Copy link

Was merged by setting zephyr's branch to this hash.

@ithinuel ithinuel closed this Jun 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.