-
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 v2.15.0 #21763
tensorflow-lite: Add v2.15.0 #21763
Conversation
This comment has been minimized.
This comment has been minimized.
Removal of 2.12.0 closes #20170 I tested this and it works (in contrast to the currently published versions 2.10 and "2.12", which fail to build) |
8756b1b
to
e0944f8
Compare
Rebased on master, now that the dependent package updates are merged. |
This comment has been minimized.
This comment has been minimized.
fe6974e
to
e8483a2
Compare
This comment has been minimized.
This comment has been minimized.
e8483a2
to
116245f
Compare
This comment has been minimized.
This comment has been minimized.
116245f
to
e9c1f90
Compare
This comment has been minimized.
This comment has been minimized.
All build failures and linter issues should be resolved now and this should be ready for review. We're using this package in production since a few months back (static build, windows and mac). |
Likewise using this on macOS, iOS, and Linux without modification. Cross-compiling to Android requires working around a problem in xnnpack/clang, but that's an upstream issue and I'll provide a separate PR to the xnnpack recipe once this one is merged. LGTM, thank you @talyz for building this integration! (I am not associated with JFrog, still needs maintainer approval.) |
e9c1f90
to
d955ca6
Compare
Just rebased on master to get the separate dependency update in. @uilianries is there anything more that needs to happen for this to be reviewed / merged? |
This comment has been minimized.
This comment has been minimized.
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.
Can be simplified or even avoided by using CMakeDeps.set_property()
on the cmake_file_name
and cmake_target_name
properties.
Authentic Vision is now using this recipe as-is in production since April with no known issues on Ubuntu 22.04, 24.04, iOS 13-17, and Android 6-14. The XNNPACK change at #22951 was recently merged to allow building for Android -- if anyone following this PR wants to do that, bump your caches :). We'd eventually want to bump TensorFlow and its dependencies to 2.15.1 and 2.16.X, would appreciate if this could be reviewed again and merged in preparation for that. |
We'll push this review forward this next week, thanks a lot for taking the time to create the PR in the first place. Could you fix the conandata conflicts meanwhile? Thanks! |
Thanks for the update @RubenRBS, the original author is @talyz though and probably received your comment with this thread anyway. I have just been using this recipe from this branch and worked on Android dependencies a bit. |
d955ca6
to
2c661e1
Compare
This comment has been minimized.
This comment has been minimized.
2c661e1
to
396fb46
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
The bot is right @talyz, Other patches have a version prefix and order in their filenames, would thus suggest to match that style for consistency. Once fixed, this should pass CI. |
396fb46
to
0354bf3
Compare
Conan v1 pipeline ✔️All green in build 5 (
Conan v2 pipeline ✔️
All green in build 5 ( |
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.
10/10 :)
Specify library name and version: tensorflow-lite/v2.15.0
Add 2.15.0 and update a few dependencies which were causing crashes when running on M1 macs under rosetta. Also, remove 2.12.0, which was just a copy of 2.10.0.
Depends on #21762, #21761, #21760 and #21759, which should be merged before this.