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

Restructure Python wheels following TF[Lite] importer changes #13483

Open
ScottTodd opened this issue May 9, 2023 · 7 comments
Open

Restructure Python wheels following TF[Lite] importer changes #13483

ScottTodd opened this issue May 9, 2023 · 7 comments
Labels
bindings/python Python wrapping IREE's C API discussion Proposals, open-ended questions, etc integrations/tensorflow TensorFlow model import and conversion

Comments

@ScottTodd
Copy link
Member

Following the work in #13037 and other issues/PRs, our TensorFlow and TensorFlow Lite importers are now pure Python packages/scripts, with no direct C++ dependencies on TensorFlow. As a result, our iree_tools_tf and iree_tools_tflite-*.whl Python wheels are now significantly simpler to build and smaller in size (~50MB before -> ~30KB after). We also no longer have a iree_tools_xla-*.whl package.

I'd like to take this chance to see if we can further simplify or restructure our released Python packages.

I was also looking at adding badges to our repo README like these:
PyPI PyPI
for those, it would be nice to have a smaller list (ideally just one package).

Some ideas:

A: Merge all iree_tools_* packages into iree_compiler

Include the Python sources for the TF/TFLite tools in the main package, but make them optional by adding checks around code like this:
https://github.com/openxla/iree/blob/060616cea34d062434097db155c458edb814e235/integrations/tensorflow/python_projects/iree_tf/iree/tools/tf/scripts/iree_import_tf/__main__.py#L60-L72

try:
  import tensorflow as tf
except ImportError as e:
  # print error / instructions for installing

Pros:

  • Just one package to install for the IREE compiler and import tools

Cons:

  • Adding TensorFlow deps (even optional ones) to the core compiler is bad layering
  • Optional imports can't enforce dependency version requirements (tf-nightly > some version)

B: Merge just iree_tools_tf and iree_tools_tflite

These packages are small and similar enough that they could be merged. If they have the same requirements (tensorflow/tf-nightly), is there any harm in merging them?

@ScottTodd ScottTodd added integrations/tensorflow TensorFlow model import and conversion discussion Proposals, open-ended questions, etc bindings/python Python wrapping IREE's C API labels May 9, 2023
@stellaraccident
Copy link
Collaborator

Option B, merging both into iree-tools-tf, seems the right thing. We can't introduce TF dependencies to the compiler.

@ScottTodd ScottTodd moved this from Inbox to Needs Scheduling in (Deprecated) IREE May 9, 2023
@hcindyl
Copy link
Contributor

hcindyl commented May 9, 2023

Sorry just caught up with this since the IREE nightly release finally has this change.

We need to update https://openxla.github.io/iree/getting-started/tflite/#using-command-line-tools if it is assumed tensorflow python package is installed. The alternative is to add tensorflow in setup.py so it is included when the wheels are installed.

The iree-import-tflite command line behavior at iree-tools-tflite-20230509.514

Traceback (most recent call last):
  File ".../iree-import-tflite", line 5, in <module>
    from iree.tools.tflite.scripts.iree_import_tflite.__main__ import main
  File ".../iree/tools/tflite/scripts/iree_import_tflite/__main__.py", line 14, in <module>
    from tensorflow.python.pywrap_mlir import experimental_tflite_to_tosa_bytecode
ModuleNotFoundError: No module named 'tensorflow'

Please advise on the proper flow to set the wheels up.

@ScottTodd
Copy link
Member Author

The current source of truth for what dependencies is required is https://github.com/openxla/iree/blob/main/integrations/tensorflow/test/requirements.txt (as of May 9 2023, that is tf-nightly==2.13.0.dev20230501). Once the Python APIs (experimental_tflite_to_tosa_bytecode and experimental_convert_saved_model_to_mlir) are stable / available in non-nightly TF releases, setup should stabilize.

@hcindyl
Copy link
Contributor

hcindyl commented May 9, 2023

The current source of truth for what dependencies is required is https://github.com/openxla/iree/blob/main/integrations/tensorflow/test/requirements.txt (as of May 9 2023, that is tf-nightly==2.13.0.dev20230501). Once the Python APIs (experimental_tflite_to_tosa_bytecode and experimental_convert_saved_model_to_mlir) are stable / available in non-nightly TF releases, setup should stabilize.

Understood. In that case, in the webdoc https://openxla.github.io/iree/getting-started/tflite/#prerequisites should also include tensorflow (or point to the requirments.txt). Is that correct?

@ScottTodd
Copy link
Member Author

Yep. Maybe we should just include tf-nightly in the recommended pip install instructions.

ScottTodd added a commit that referenced this issue May 11, 2023
Follow-up to #13068. This path has
been unused, and IREE natively supports importing from StableHLO now.
The `iree-tools-tf` and `iree-tools-tflite` packages still exist, though
they are now tiny pure python packages that could be merged into a
single package (see #13483).
monorimet pushed a commit to nod-ai/SRT that referenced this issue May 12, 2023
Follow-up to iree-org#13068. This path has
been unused, and IREE natively supports importing from StableHLO now.
The `iree-tools-tf` and `iree-tools-tflite` packages still exist, though
they are now tiny pure python packages that could be merged into a
single package (see iree-org#13483).
monorimet pushed a commit to nod-ai/SRT that referenced this issue May 12, 2023
Follow-up to iree-org#13068. This path has
been unused, and IREE natively supports importing from StableHLO now.
The `iree-tools-tf` and `iree-tools-tflite` packages still exist, though
they are now tiny pure python packages that could be merged into a
single package (see iree-org#13483).
monorimet pushed a commit to nod-ai/SRT that referenced this issue May 15, 2023
Follow-up to iree-org#13068. This path has
been unused, and IREE natively supports importing from StableHLO now.
The `iree-tools-tf` and `iree-tools-tflite` packages still exist, though
they are now tiny pure python packages that could be merged into a
single package (see iree-org#13483).
monorimet pushed a commit to nod-ai/SRT that referenced this issue May 15, 2023
Follow-up to iree-org#13068. This path has
been unused, and IREE natively supports importing from StableHLO now.
The `iree-tools-tf` and `iree-tools-tflite` packages still exist, though
they are now tiny pure python packages that could be merged into a
single package (see iree-org#13483).
sleffler pushed a commit to AmbiML/sparrow-scripts-full that referenced this issue Jun 7, 2023
Addding specific tensorflow package due to
iree-org/iree#13483.

Change-Id: I3458ec9b72442e4efd03772d78f317c3c0ffdc03
GitOrigin-RevId: 8d1e2dc74e2f2a95a43e80280a6b75cd24cd857d
NatashaKnk pushed a commit to NatashaKnk/iree that referenced this issue Jul 6, 2023
Follow-up to iree-org#13068. This path has
been unused, and IREE natively supports importing from StableHLO now.
The `iree-tools-tf` and `iree-tools-tflite` packages still exist, though
they are now tiny pure python packages that could be merged into a
single package (see iree-org#13483).
@allieculp allieculp moved this from Needs Scheduling to Backlog in (Deprecated) IREE Aug 29, 2023
@ScottTodd
Copy link
Member Author

I'm wondering if we'll want to keep the iree-tools-tf and iree-tools-tflite packages separate now that TensorFlow Lite got renamed to LiteRT back in September: https://developers.googleblog.com/en/tensorflow-lite-is-now-litert/. The TFLite python packages got replaced with https://pypi.org/project/ai-edge-litert/ and code got moved from TF to https://github.com/google-ai-edge/litert.

I haven't seen or heard that much about either framework lately, but TFLite/LiteRT is still in use in a few spots (like #19402).

@ScottTodd
Copy link
Member Author

I'm now thinking option A ("Merge all iree_tools_* packages into iree_compiler") could work well, using extras like [tf] and [litert]. We have some precedent with the [onnx] extra and the iree-import-onnx tool. The deps at this point are all optional Python dependencies, not C++ source dependencies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bindings/python Python wrapping IREE's C API discussion Proposals, open-ended questions, etc integrations/tensorflow TensorFlow model import and conversion
Projects
None yet
Development

No branches or pull requests

3 participants