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

pkg/tensorflow-lite: add support to RIOT #12847

Merged
merged 6 commits into from
Jan 15, 2020

Conversation

aabadie
Copy link
Contributor

@aabadie aabadie commented Nov 29, 2019

Contribution description

This PR adds support for TensorFlow Lite for microcontrollers to RIOT.

It based on 2 PRs

The integration into the build system can certainly be improved, so comments are welcome, of course.

The provided test application is just a plain copy (with .cc extensions changed into .cpp) of the hello-world sample, but there are more and it shouldn't be too difficult to adapt them.

cc'ing @adjih and @emmanuelsearch, who might be interested in testing this.

Testing procedure

Just build and run the test application:

make BOARD=<board of your choice> -C tests/pkg_tensorflow-lite flash term

It takes a bit of time to build but it's worth waiting :)

Issues/PRs references

Based on #12846 and #12844

@aabadie aabadie added Type: new feature The issue requests / The PR implemements a new feature for RIOT Area: pkg Area: External package ports labels Nov 29, 2019
@aabadie aabadie force-pushed the pr/pkg/tensorflow-lite branch 2 times, most recently from 8e9fa93 to 2c1fbcf Compare November 29, 2019 21:26
@aabadie
Copy link
Contributor Author

aabadie commented Dec 2, 2019

I pushed some code that adapts the magic_wand example but couldn't make it work yet: the interpreter remains stuck when calling it Invoke method (I'm still investigating).
The accelerometer handler uses the lsm6dsl sensor instead of the lis2dh12 (SparkFun) and LSM9DS1 (Arduino) used by the upstream examples.

@fjmolinas fjmolinas added this to the Release 2020.01 milestone Dec 13, 2019
@fjmolinas fjmolinas added the State: waiting for other PR State: The PR requires another PR to be merged first label Dec 13, 2019
@aabadie
Copy link
Contributor Author

aabadie commented Dec 17, 2019

the interpreter remains stuck when calling it Invoke method (I'm still investigating).

I found the problem: the stack size of the main thread was not large enough. Doubling it and the workflow now works. Unfortunately, I can't get a valid gesture detection.
I'll try to update to a more recent commit of the package since it seems there were some updates recently.

@emmanuelsearch
Copy link
Member

@aabadie could it be that the gesture model is (highly) dependent on the actual accelerometer type?

@emmanuelsearch
Copy link
Member

And the new URL for the examples upstream are https://github.com/tensorflow/tensorflow/tree/master/tensorflow/lite/micro/examples

@aabadie
Copy link
Contributor Author

aabadie commented Dec 17, 2019

could it be that the gesture model is (highly) dependent on the actual accelerometer type?

It could be several things. In the examples, they are using different accelerometers (all ST though) and use the FIFO mode, which RIOT doesn't provide. Maybe the range of data is not the same.

There was no model training example for the magic wand in the TensorFlow version used by this PR. It was added recently so it may help training a model that best fit the RIOT accelerometer data.

@aabadie
Copy link
Contributor Author

aabadie commented Dec 17, 2019

I updated the package version and adapted to the new structure (was quite easy) but still no luck with the magic wand. Maybe it's time to train some model on the sensor data I'm using ? ;)

@aabadie
Copy link
Contributor Author

aabadie commented Dec 20, 2019

I managed to get valid prediction with the magic wand model:

2019-12-20 09:07:07,931 # main(): This is RIOT! (Version: 2020.01-devel-1453-g4c8384-pr/pkg/tensorflow-lite)
2019-12-20 09:07:07,954 # Magic starts!
2019-12-20 09:07:21,134 # WING:
2019-12-20 09:07:21,136 # *         *         *
2019-12-20 09:07:21,138 #  *       * *       *
2019-12-20 09:07:21,140 #   *     *   *     *
2019-12-20 09:07:21,142 #    *   *     *   *
2019-12-20 09:07:21,144 #     * *       * *
2019-12-20 09:07:21,146 #      *         *
2019-12-20 09:07:21,146 # 

It's still very inaccurate and requires changes in the lsm6dsl driver. I think having the possibility to use the FIFO mode of the driver would also help.

@aabadie
Copy link
Contributor Author

aabadie commented Dec 30, 2019

Finally, I replaced the magic_wand example with a new one based on the MNIST hand-written digit. The new example comes with Python scripts to train the model and to generate the input digit used by the application. This new example replicates the application of the utensor package.

Regarding the magic wand, I'll provide it as a follow-up because it requires other changes in RIOT ST accelerrometer drivers (read data as float, require FIFO mode support). So no need to block this PR because of this.

@adjih
Copy link
Contributor

adjih commented Dec 30, 2019

Note that the magic wand example is not about drawing a letter with the hand. It is about doing a gesture with an Harry Potter-like wand, and was inspired by J. Wang's example .

Indeed FIFO support can be skipped: if one is really interested into this application (for next Halloween, etc.), one can train with a custom dataset, and using provided CNN/LTSM training ).

@aabadie aabadie force-pushed the pr/pkg/tensorflow-lite branch 3 times, most recently from b05df83 to d2e5cc1 Compare January 5, 2020 14:21
@aabadie aabadie added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jan 5, 2020
@aabadie aabadie force-pushed the pr/pkg/tensorflow-lite branch 3 times, most recently from a01ff1c to 925b570 Compare January 9, 2020 11:57
@aabadie
Copy link
Contributor Author

aabadie commented Jan 10, 2020

Tensorflow released the 2.1 version this week. I'll check if this PR can be based on it, instead of using a random commit in master.

@aabadie
Copy link
Contributor Author

aabadie commented Jan 10, 2020

So I gave a try to the 2.1 version and it seems to not contain the latest changes that are used in this PR.

@aabadie aabadie removed the State: waiting for other PR State: The PR requires another PR to be merged first label Jan 13, 2020
@aabadie
Copy link
Contributor Author

aabadie commented Jan 13, 2020

@fjmolinas, rebased now that #12846 is merged. Maybe I should squash since there was no review until now ?

@aabadie aabadie force-pushed the pr/pkg/tensorflow-lite branch 2 times, most recently from 1eb9b23 to 785d6ca Compare January 13, 2020 12:45
@aabadie
Copy link
Contributor Author

aabadie commented Jan 13, 2020

I got a green light from @fjmolinas IRL, so squashed :)

@aabadie aabadie force-pushed the pr/pkg/tensorflow-lite branch 2 times, most recently from b4eec04 to 1b61e4f Compare January 13, 2020 15:44
Copy link
Contributor

@fjmolinas fjmolinas left a comment

Choose a reason for hiding this comment

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

Some style comments.

pkg/tensorflow-lite/Makefile Show resolved Hide resolved
pkg/tensorflow-lite/Makefile.include Outdated Show resolved Hide resolved
pkg/tensorflow-lite/Makefile.tensorflow-lite-c Outdated Show resolved Hide resolved
pkg/tensorflow-lite/Makefile.tensorflow-lite-core Outdated Show resolved Hide resolved
pkg/tensorflow-lite/Makefile.tensorflow-lite-core Outdated Show resolved Hide resolved
tests/pkg_tensorflow-lite/Makefile Show resolved Hide resolved
tests/pkg_tensorflow-lite/README.md Outdated Show resolved Hide resolved
tests/pkg_tensorflow-lite/main.cpp Show resolved Hide resolved
Copy link
Contributor

@fjmolinas fjmolinas left a comment

Choose a reason for hiding this comment

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

Tested generating a new image (digit 1) and code correctly detected it, the test script doesn't work though since the digit detection is hardcoded:

main(): This is RIOT! (Version: 2020.01-devel-1738-g1b61e-pr-12847)
Digit prediction: 1
Timeout in expect script at "child.expect_exact("Digit prediction: 7")" (tests/pkg_tensorflow-lite/tests/01-run.py:8)

@aabadie
Copy link
Contributor Author

aabadie commented Jan 14, 2020

@fjmolinas, I think I addressed all your comments. Let me know if it's ok for you now!

fjmolinas
fjmolinas previously approved these changes Jan 14, 2020
Copy link
Contributor

@fjmolinas fjmolinas left a comment

Choose a reason for hiding this comment

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

@aabadie one last comment please squash after amending.

pkg/gemmlowp/Makefile.gemmlowp Outdated Show resolved Hide resolved
@fjmolinas fjmolinas dismissed their stale review January 14, 2020 21:12

Wrong button, ups

Copy link
Contributor

@fjmolinas fjmolinas left a comment

Choose a reason for hiding this comment

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

ACK!

@aabadie aabadie merged commit 73d68c7 into RIOT-OS:master Jan 15, 2020
@aabadie aabadie deleted the pr/pkg/tensorflow-lite branch January 15, 2020 05:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: pkg Area: External package ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants