-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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: add version 2.14.0 #20173
Conversation
This comment has been minimized.
This comment has been minimized.
3170b86
to
e0d8daa
Compare
Conan v1 pipeline ❌Sorry, the build is only launched for Access Request users. You can request access writing in this issue. Conan v2 pipeline ❌
The v2 pipeline failed. Please, review the errors and note this is required for pull requests to be merged. In case this recipe is still not ported to Conan 2.x, please, ping See details:Sorry, the build is only launched for Access Request users. You can request access writing in this issue. |
- patch_file: "patches/disable_fetch_content.patch" | ||
patch_description: "Fail if the CMake build script tries to fetch external dependencies" | ||
patch_type: "conan" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This patch can probably be replaced with simply setting FETCHCONTENT_FULLY_DISCONNECTED
.
-find_package(fft2d REQUIRED) | ||
+find_package(fft REQUIRED) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can probably do deps = ConanDeps(self); deps.set_property("fft", "cmake_file_name", "fft2d")
instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same goes for xnnpack.
- farmhash | ||
+ farmhash::farmhash | ||
fft2d_fftsg2d | ||
- flatbuffers::flatbuffers | ||
- gemmlowp::gemmlowp | ||
- ml_dtypes | ||
+ ${FLATBUFFERS_TARGET} | ||
+ gemmlowp::eight_bit_int_gemm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
deps.set_property("farmhash", "cmake_target_name", "farmhash")
deps.set_property("gemmlowp", "cmake_target_name", "gemmlowp::gemmlowp")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At this point I would rather download()
it from GitHub instead. That's a bit excessive for a patch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The full header file is a tad excessive for a patch. Patching can probably simplified slightly. Otherwise LGTM.
Thanks!
Thank you for the review. Unfortunately I'm not working anymore on that project, so I do not have the setup to test the modification I could make with you remarks. |
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
This pull request has been automatically closed because it has not had recent activity. Thank you for your contributions. |
Library name and version: tensorflow-lite/2.14.0
Add new version 2.14.0 of Tensorflow lite.
Modifications compare to tensorflow repo:
ml_dtypes
to be able to compile.Conan package modification:
test_package
.