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

Can't bump tflite-micro: new "PIL" (Pillow) Python dependency #230

Open
tcal-x opened this issue Aug 15, 2021 · 9 comments
Open

Can't bump tflite-micro: new "PIL" (Pillow) Python dependency #230

tcal-x opened this issue Aug 15, 2021 · 9 comments

Comments

@tcal-x
Copy link
Collaborator

tcal-x commented Aug 15, 2021

I tried bumping the tflite-micro submodule to current main, and encounter "No module named 'PIL'" when building software.

I don't think we have a precedent of using pip to install a missing Python package. I tried cloning https://github.com/python-pillow/Pillow under third_party/python/, but it was not found. It may be that the Pillow repository is structured a bit differently than other Python repos -- there is a PIL directory at Pillow/src/PIL.

Or perhaps we're currently not setting up PYTHONPATH for the tflite-micro build.

@danc86
Copy link
Collaborator

danc86 commented Aug 15, 2021

PIL has some C bits that need compiling, which is why it's just not a plain Python package that you can clone and import.

Maybe the best option is to just apt install python3-pil?

@tcal-x
Copy link
Collaborator Author

tcal-x commented Aug 16, 2021

Thanks @danc86 , I thought that would be the answer! But it didn't work as expected on my Ubuntu 20.04LTS laptop nor on my work laptop. I tried pip3 install Pillow==7.1.2 on my personal laptop, and it got me past this issue to the next issue (a complaint about not including the correct header for printf).

Edit: the printf issue is easily remedied by adding #include <cstdio> to print_params.cc.

@tcal-x
Copy link
Collaborator Author

tcal-x commented Aug 17, 2021

After working around the above two issues, I encounter this issue:

make[1]: Entering directory '/home/tim/tcal-x/CFU-Playground/proj/proj_template_v/build'
  CXX reduce.cc reduce.o
In file included from /home/tim/tcal-x/CFU-Playground/proj/proj_template_v/build/src/tensorflow/lite/kernels/internal/common.h:29,
                 from /home/tim/tcal-x/CFU-Playground/proj/proj_template_v/build/src/tensorflow/lite/kernels/internal/reference/reduce.h:19,
                 from /home/tim/tcal-x/CFU-Playground/proj/proj_template_v/build/src/tensorflow/lite/micro/kernels/reduce.cc:16:
/home/tim/tcal-x/CFU-Playground/proj/proj_template_v/build/src/tensorflow/lite/kernels/internal/types.h: In function 'TfLiteStatus tflite::ops::micro::reduce::EvalMean(TfLiteContext*, TfLiteNode*)':
/home/tim/tcal-x/CFU-Playground/proj/proj_template_v/build/src/tensorflow/lite/kernels/internal/types.h:185:33: error: 'resolved_axis' may be used uninitialized in this function [-Werror=maybe-uninitialized]
         if (idx == axis[axis_idx]) {
                    ~~~~~~~~~~~~~^
/home/tim/tcal-x/CFU-Playground/proj/proj_template_v/build/src/tensorflow/lite/kernels/internal/types.h:185:33: error: 'resolved_axis' may be used uninitialized in this function [-Werror=maybe-uninitialized]
         if (idx == axis[axis_idx]) {
                    ~~~~~~~~~~~~~^
/home/tim/tcal-x/CFU-Playground/proj/proj_template_v/build/src/tensorflow/lite/kernels/internal/types.h:185:33: error: 'resolved_axis' may be used uninitialized in this function [-Werror=maybe-uninitialized]
         if (idx == axis[axis_idx]) {
                    ~~~~~~~~~~~~~^
cc1plus: all warnings being treated as errors
make[1]: *** [Makefile:188: /home/tim/tcal-x/CFU-Playground/proj/proj_template_v/build/src/tensorflow/lite/micro/kernels/reduce.o] Error 1
make[1]: Leaving directory '/home/tim/tcal-x/CFU-Playground/proj/proj_template_v/build'
make: *** [../proj.mk:190: /home/tim/tcal-x/CFU-Playground/proj/proj_template_v/build/software.bin] Error 2

@mithro
Copy link
Contributor

mithro commented Oct 19, 2021

How are you installing the Python dependencies? You should just need to add it to your requirements.txt or environment.yml file?

@tcal-x
Copy link
Collaborator Author

tcal-x commented Oct 19, 2021

How are you installing the Python dependencies? You should just need to add it to your requirements.txt or environment.yml file?

Yes, when we switch to Conda* installation, that is what we will do. Currently, we explicitly set PYTHONPATH to include everything under third_party/python/. For example, to handle the new meson dependency from picolibc, we added meson as a submodule (ad5cbfc) instead of adding it to a requirements.txt file.

* Two of the barriers have been addressed (differences in performance Conda vs fresh-built, and lack of a RISCV toolchain package with g++), so we should be able to move forward with using Conda / make-env now.

@mithro
Copy link
Contributor

mithro commented Oct 19, 2021

As you have just discovered, manipulating the PYTHONPATH is not a support way for adding modules into your Python environment. It breaks in a huge number of ways that I'm surprised you are only seeing this issue now.

@alanvgreen
Copy link
Collaborator

@mithro All the mechanisms for setting up environments have interesting shortcomings. The /environment script we have is not how I would have chosen to do it, but it's been working well. Perhaps we could discuss replacing it in person or on a different issue?

@alanvgreen
Copy link
Collaborator

@tcal-x In terms of timing I think we'll be happy with the current TfLM version for the next six months or so, so happy to wait while the environment issues are sorted out.

@mithro
Copy link
Contributor

mithro commented Oct 21, 2021

The three primary options for Python environments are;

  • (a) System install (IE using packages / pip and/or pip --user). No isolation from what the user has previously installed.
  • (b) "Python-only" virtualenv system (couple of choices being virtualenv, venv and pipenv). Creates a "self contained" environment containing Python. Anything not in directly in Python is provided by the system. Good for pure Python systems.
  • (c) Conda. A self contained environment containing Python and other packages which would be provided by the system otherwise. Good for systems which want to support multiple platforms (Linux, Windows, Mac).

In both (b) and (c) any locally provided dependencies (like through submodules) need to get "installed" into the environment.

In (a) any locally provided dependencies (like through submodules) need to get installed onto the users system. However, people hate doing this so they slowly end up re-creating either (b) or (c) poorly (I know because I've fallen into this trap myself).

It is actually pretty easy to start with (b) and then add / extend to (c). I have some notes around this topic here.

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

No branches or pull requests

4 participants