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

tensorflow-lite fixes #85

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

tensorflow-lite fixes #85

wants to merge 2 commits into from

Conversation

vivien
Copy link
Contributor

@vivien vivien commented Aug 15, 2023

This PR makes it possible to compile libcamera-apps from meta-raspberrypi with tflite support.

vivien added 2 commits August 15, 2023 16:33
Do not install the tensorflow-lite 2.* library as tensorflow2-lite
because packages will expect libtensorflow-lite.a and
tensorflow-lite.pc. This also allows us to use the same pkg-config
file.

Signed-off-by: Vivien Didelot <[email protected]>
Add mickledore to the list of supported version.

Signed-off-by: Vivien Didelot <[email protected]>
@taos-ci
Copy link
Collaborator

taos-ci commented Aug 15, 2023

📝 TAOS-CI Version: 1.5.20200925. Thank you for submitting PR #85. Please a submit 1commit/1PR (one commit per one PR) policy to get comments quickly from reviewers. Your PR must pass all verificiation processes of cibot before starting a review process from reviewers. If you are new member to join this project, please read manuals in documentation folder and wiki page. In order to monitor a progress status of your PR in more detail, visit http://ci.nnstreamer.ai/.

@taos-ci
Copy link
Collaborator

taos-ci commented Aug 15, 2023

:octocat: cibot: @vivien, A builder checker could not be completed because one of the checkers is not completed. In order to find out a reason, please go to http://ci.nnstreamer.ai/meta-neural-network/ci/repo-workers/pr-checker/85-0-664b/.

@@ -13,7 +13,7 @@ SRC_URI = " \
file://0001-change-flatbuffers-version.patch \
file://0001-build-Add-bundle-static-lib-script-in-CMakeLists.txt.patch \
file://0001-build-Remove-mcpu-flag-when-build-XNNPACK.patch \
file://tensorflow2-lite.pc.in \
file://tensorflow-lite.pc.in \
Copy link
Member

Choose a reason for hiding this comment

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

We have a few systems that install both tf-lite 1.x and tf2-lite 2.x, which forced us to install this file as tensorflow2-lite.pc.

For your needs, I'd recommend to make a "meta package" that creates symbolic links from tensorflow2-lite* to tensorflow-lite* for the compatibility of both tensorflow2* users and tensorflow* users.

Copy link
Contributor Author

@vivien vivien Aug 17, 2023

Choose a reason for hiding this comment

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

I replied in the PR conversation for clarity ;)

Copy link
Member

@myungjoo myungjoo left a comment

Choose a reason for hiding this comment

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

This breaks packages using tensorflow2-lite.*

Please consider creating symbolic links from tensorflow2* to tensorflow* optionally or by another package.

@vivien vivien closed this Aug 17, 2023
@vivien vivien reopened this Aug 17, 2023
@vivien
Copy link
Contributor Author

vivien commented Aug 17, 2023

For your needs, I'd recommend to make a "meta package" that creates symbolic links from tensorflow2-lite* to tensorflow-lite* for the compatibility of both tensorflow2* users and tensorflow* users.

I added the following to the concerned layer (here meta-raspberrypi) and this works fine:

$ cat meta-raspberrypi/dynamic-layers/neuralnetwork/recipes-tensorflow/tensorflow-lite/tensorflow-lite_2.%.bbappend 
do_install:append () {
	ln -sf libtensorflow2-lite.a ${D}${libdir}/libtensorflow-lite.a
	ln -sf tensorflow2-lite.pc ${D}${libdir}/pkgconfig/tensorflow-lite.pc
}

But I do not think that requiring this for all layers using tensorflow-lite is the correct packaging approach, see below.

We have a few systems that install both tf-lite 1.x and tf2-lite 2.x, which forced us to install this file as tensorflow2-lite.pc.

I understand your use case, however the package seems broken for a few reasons:

  • the project builds libtensorflow-lite-bundled.a, not libtensorflow2-lite-bundled.a
  • headers are installed into ${includedir}/tensorflow/lite, regardless of the version
  • ${includedir}/tensorflow/core/public/version.h is installed regardless of the version
  • ${includedir}/flatbuffers/include/flatbuffers/*.h are installed regardless of the version

The r2.* branches are just a version bump from the tensorflow-lite project, not a different project.

Because you have a special scenario, I would suggest that you install the various versions in different prefixes on your system (e.g. /opt/tflite/1.12, /opt/tflite/1.15, /opt/tflite/2.3, /opt/tflite/2.11, etc.) and configure your system or applications to look for the correct version there.

A way to do that is to have a dynamic tensorflow-lite_%.bbappend that is enabled only for your use case, which moves the installed files to /opt/tflite/${PV} for example.

What do you think?

vivien added a commit to vivien/meta-raspberrypi that referenced this pull request Sep 21, 2023
The tensorflow-lite_2.* recipe from meta-neural-network installs
the library and pkg-config files as tensorflow2-lite*, because their
users may use several versions of the same library.

However packages such as libcamera-apps expect tensorflow-lite* files.

While this is being discussed with the upstream meta-neural-network
layer, adding a dynamic bbappend in meta-raspberrypi also helps
documenting the support for this tensorflow-lite provider.

Refs nnstreamer/meta-neural-network#85

Signed-off-by: Vivien Didelot <[email protected]>
vivien added a commit to vivien/meta-raspberrypi that referenced this pull request Sep 21, 2023
The tensorflow-lite_2.* recipe from meta-neural-network installs
the library and pkg-config files as tensorflow2-lite*, because their
users may use several versions of the same library.

However packages such as libcamera-apps expect tensorflow-lite* files.

While this is being discussed with the upstream meta-neural-network
layer, adding a dynamic bbappend in meta-raspberrypi also helps
documenting the support for this tensorflow-lite provider.

Refs nnstreamer/meta-neural-network#85

Signed-off-by: Vivien Didelot <[email protected]>
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.

3 participants