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/cmsis-nn: add support to RIOT #13062

Merged
merged 2 commits into from
Feb 14, 2020
Merged

Conversation

aabadie
Copy link
Contributor

@aabadie aabadie commented Jan 9, 2020

Contribution description

This PR adds support for the neural network API provided by ARM in the CMSIS repository.
A test application is provided for performing the detection of an object in an RGB image from the CIFAR10 dataset. This sample application is adapted from the provided example in the upstream repo.
The image is embedded in the generated firmware using the BLOB mechanism.

Note that the package could have been done based on the existing cmsis-dsp but I wanted to use the latest release 5.6 and the existing cmsis-dsp is using 5.4 and doesn't seem to be that easy to update.
I already have a branch that does that but it's not working on all supported CortextM (like M0).

cc @emmanuelsearch, as we already talked about that before.

Testing procedure

The following command should succeed:

make BOARD=<one of the whitelisted board> -C tests/pkg_cmsis-nn flash test

Issues/PRs references

None

@aabadie aabadie added Platform: ARM Platform: This PR/issue effects ARM-based platforms 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 labels Jan 9, 2020
@benpicco benpicco added the Reviewed: 3-testing The PR was tested according to the maintainer guidelines label Feb 12, 2020
Copy link
Contributor

@benpicco benpicco left a comment

Choose a reason for hiding this comment

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

The example code might as well be black magic, but I see it was already as opaque in the original example.
I'm impressed by how fast it is even without any hardware acceleration.

# Required for some basic math functions
INCLUDES += -I$(PKGDIRBASE)/cmsis-nn/CMSIS/DSP/Include

CFLAGS += -Wno-sign-compare
Copy link
Contributor

Choose a reason for hiding this comment

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

already in Makefile

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I kept this one and removed the other: the warning occurs in a header file and is not silenced from the package Makefile. So I have to silent it globally unfortunately.

@benpicco
Copy link
Contributor

Looks good to me - please squash.

Copy link
Contributor

@benpicco benpicco left a comment

Choose a reason for hiding this comment

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

Package works fine with example faithfully converted from upstream C++ example.

@aabadie
Copy link
Contributor Author

aabadie commented Feb 14, 2020

Squashed!

@benpicco benpicco merged commit 5d940af into RIOT-OS:master Feb 14, 2020
@aabadie aabadie deleted the pr/pkg/cmsis-nn branch February 14, 2020 15:50
@aabadie
Copy link
Contributor Author

aabadie commented Feb 14, 2020

Thanks for reviewing and merging @benpicco !

@leandrolanzieri leandrolanzieri added this to the Release 2020.04 milestone Feb 20, 2020
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 Platform: ARM Platform: This PR/issue effects ARM-based platforms Reviewed: 3-testing The PR was tested according to the maintainer guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants