nengo / nengo-dl

Deep learning integration for Nengo
https://www.nengo.ai/nengo-dl
Other
88 stars 22 forks source link

Fix tensorflow-gpu installation check #99

Closed drasmuss closed 5 years ago

drasmuss commented 5 years ago

Adding a pyproject.toml switched us over into pep517-style pip installation, which means that setup.py gets executed within an isolated build environment. This makes it difficult to check whether or not tensorflow is installed, which breaks our conditional logic in setup.py which would attempt to look for a pre-existing tensorflow-gpu installation. This means that our setup.py was installing tensorflow overtop of an existing tensorflow-gpu installation, which breaks the GPU support.

There are two fixes here. The first is a kind of hacky fix that attempts to find the original (non-isolated) site-packages directory and then look in there for tensorflow-gpu. It's hacky, but it preserves all the same functionality as before.

The second fix takes advantage of a change in the most recent version of TensorFlow (1.14), which allows the tensorflow-gpu package to be used whether or not GPU support is actually available (if it isn't available, you just end up with CPU-only tensorflow). So we can just require tensorflow-gpu>=1.14 in all cases, and it will always work. This has the advantage of simplicity, but it is a slight downgrade from a user experience. 1) It won't play nicely with tf-nightly* packages, 2) it will force tensorflow-gpu to be installed even if the user has an existing tensorflow (non-gpu) installation.

tbekolay commented 5 years ago

The second solution seems better to me:

  1. People who are using the tf-nightly* packages will likely have the expertise to do pip install nengo-dl --no-deps and then install the other requirements manually (are there other requirements?)
  2. If they have an existing install, it is likely that it's an earlier version than 1.14 and so they would have to upgrade anyway. And upgrading the CPU-only install should be easier than the GPU install (?)
drasmuss commented 5 years ago

Actually it looks like the TF1.14 dynamic GPU support doesn't work on windows (see the failing appveyor build). So I'll go with the first, hacky fix for now, with the hope that in the near-ish future we can switch to something like the second fix once that is fully supported in TensorFlow.