tflite-micro icon indicating copy to clipboard operation
tflite-micro copied to clipboard

tflite_micro python package with signal ops

Open ericheim opened this issue 1 year ago • 13 comments

BUG=#2357

Description

The proposed solution includes signal_ops to the tflite_micro python package. Adding signal_ops introduces a linker dependency to the specific tensorflow-cpu version the tflite_micro package was build against. E.G. when the python package was build against tensorflow-cpu==2.12.0, the version that is used in the requirements.txt, the signal ops will only be able to work when tensorflow=2.12.0 is installed on the system. Tested the build with up to version 2.15.0

Possible improvements

I added the tensorflow target version (2.12.0) to the requirements of the WHEEL and third_party/python_requirements.in. This should probably be done somewhere at a central place. Feedback is welcomed.

ericheim avatar Dec 15 '23 14:12 ericheim

Does the CI require a rebase instead of a merge?

ericheim avatar Dec 15 '23 15:12 ericheim

Does the CI require a rebase instead of a merge?

No, merge is fine.

rascani avatar Dec 15 '23 17:12 rascani

@ericheim - Just as an FYI, I'm interested in merging this, just working out the ramifications of pinning against a specific version of the tensorflow.

rascani avatar Dec 19 '23 19:12 rascani

Hi @rkuester , @rascani Thanks for your feedback!

@rascani The pinning of the tensorflow version can probably be done somehwere in a central place. @rkuester import paths in the example tflite-micro/tensorflow/lite/examples/micro_speech can also be changed after adopting the __init__.py.

ericheim avatar Dec 20 '23 19:12 ericheim

The pinning of the tensorflow version can probably be done somehwere in a central place.

Understood, but we have to consider that not all users will be using the same tensorflow version. As I understand it, TF doesn't have any ABI stability guarantees between minor versions (and we're depending on C++ code), so this essentially requires all users to use 2.12. That might be okay, but we need to figure out what the policy should be for this version.

rascani avatar Dec 20 '23 20:12 rascani

Hi @rkuester , @rascani ,

I extended the tflite_micro.postinstall_check.passed() with the signal library. Unfortunately adapting the import paths in the __init__.py is a bigger change than expected. This is due to missing __init__.py inside of tflite_micro/python and tflite_micro/python/tflite_micro in the WHEEL file. Using it in dev mode doesn't has these problems.

Adding more code into the existing init files, e.g. directly adding the directories to sys.path interferes either with the tensorflow installation signal ops or breaks the tflite_micro in dev mode, when using it from the folder directly.

ericheim avatar Dec 21 '23 18:12 ericheim

Thanks for adding the test, @ericheim.

I'm working on the import name issue. You're right, adding a subpackage to the public API via the __init__.py trick doesn't work the same way that adding a module works.

rkuester avatar Dec 30 '23 00:12 rkuester

@rkuester Thanks for taking care of the import name issue!

ericheim avatar Jan 02 '24 12:01 ericheim

"This PR is being marked as stale due to inactivity. Remove label or comment to prevent closure in 5 days."

github-actions[bot] avatar Feb 12 '24 10:02 github-actions[bot]

"This PR is being closed because it has been marked as stale for 5 days with no further activity."

github-actions[bot] avatar Feb 18 '24 10:02 github-actions[bot]

"This PR is being marked as stale due to inactivity. Remove label or comment to prevent closure in 5 days."

github-actions[bot] avatar Apr 01 '24 10:04 github-actions[bot]

Resolved merge conflict. The solution still has to be pinned against a specific tensorflow version.

ericheim avatar Apr 03 '24 12:04 ericheim

"This PR is being marked as stale due to inactivity. Remove label or comment to prevent closure in 5 days."

github-actions[bot] avatar May 14 '24 10:05 github-actions[bot]

"This PR is being marked as stale due to inactivity. Remove label or comment to prevent closure in 5 days."

github-actions[bot] avatar Jun 24 '24 10:06 github-actions[bot]

"This PR is being closed because it has been marked as stale for 5 days with no further activity."

github-actions[bot] avatar Jun 30 '24 10:06 github-actions[bot]