Combine python/setup.py.in into setup.py

[danielhers]

Simplify Python installation process by combining the generated setup.py into the top one, using environment variables to pass information from cmake. Should allow fixing #657 now that the Cython extensions are created by the main setup.py.

[pmichel31415]

Does this work with .rst files?

[danielhers]

You mean the bit I removed? I suppose, as it came from the .rst documentation. But this is Markdown...

[pmichel31415]

OK we'll see, if it bugs I'll correct it after the pr is merged

[neubig]

Thanks a lot for this! It seems to be breaking the build in my environment. Attached are the cmake and make log files noted here:
http://dynet.readthedocs.io/en/latest/debugging.html#asking-questions-reporting-bugs

The actual error I'm getting is when doing the Python cmake:

cd /home/gneubig/work/dynet/buildsetup/python && /usr/bin/cmake -E env CMAKE_INSTALL_PREFIX="/usr/local" PROJECT_SOURCE_DIR="/home/gneubig/work/dynet" PROJECT_BINARY_DIR    ="/home/gneubig/work/dynet/buildsetup" LIBS="-lpthread" EIGEN3_INCLUDE_DIR="/usr0/home/gneubig/usr/local/eigen" MKL_LINK_DIRS="" WITH_CUDA_BACKEND="" CUDA_RT_FILES="" CU    DA_RT_DIRS="" CUDA_CUBLAS_FILES="" CUDA_CUBLAS_DIRS="" /home/gneubig/usr/local/anaconda3/envs/throwaway/bin/python /home/gneubig/work/dynet/setup.py build_ext --inplace
CMake Error: cmake version 2.8.12.2
Usage: /usr/bin/cmake -E [command] [arguments ...]

Any ideas here?

[danielhers]

Apparently cmake -E env is only available from version 3.1, and you have 2.8.12.2. I guess I'll have to find another way to pass these variables.

[danielhers]

@neubig Fixed this issue. Could you try again please?

[neubig]

@danielhers Thanks! I took a look at this now. A couple questions:

1. I specified a non-standard path to Eigen during the cmake configuration step, but it seems that this is ignored by setup.py, which resulted in a new version of Eigen being cloned with mercurial and everything being recompiled. Is there a way for setup.py to inherit the settings from cmake?
2. Or alternatively and preferably, is calling cmake explicitly even necessary anymore? Would it possible to do everything in setup.py? If so, this would simplify things a lot.

[danielhers]

@neubig I suppose you are doing the "manual" installation. If you do pip install . then you won't have to run cmake yourself, since setup.py already does everything on its own!

If you are doing the manual installation, you still need to run setup.py at the end to install the Python bindings (as before, but it's no longer in build/python but rather in the root directory). Note that you must give it the build directory path so it knows you already built DyNet and doesn't try to build it again (I updated this in the docs as part of the PR):

cd build/python
python ../../setup.py build --build-dir=.. install --user

Note that setup.py looks at the EIGEN3_INCLUDE_DIR environment variable to determine where to save eigen, and if already exists, then it leaves it as is. If you did the manual installation and specified -DEIGEN3_INCLUDE_DIR to cmake, you should also define it as an environment variable with the same value before running setup.py at the end, or else add it to the command line, e.g.:

python ../../setup.py EIGEN3_INCLUDE_DIR=/usr/local/include/eigen build --build-dir=.. install --user

[neubig]

@danielhers Thanks for the clarification! I am indeed following the manual install instructions, because I need them as a developer (and also because I need CUDA, which is not supported with pip at the moment AFAIK).

I tried the manual install instructions again in a clean environment and followed the instructions word-for-word. It looks like pip is still pulling Eigen a second time and recompiling everything when I run setup.py. Perhaps this is because the instructions pull a particular version (346ecdb) of Eigen, then setup.py is pulling the latest version? Here are the relevant parts of the log:

INFO:root:Cloning Eigen...
destination directory: eigen
applying clone bundle from https://media-api.atlassian.io/file/9cd0da8e-8d40-4d63-8a5a-1ed01de3f5a1/binary?token=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiI1MTRmMWY1ZC02YmRmLTQ2YmMtYmVhZC1hMTY4NTE2NzFmYTciLCJhY2Nlc3MiOnsidXJuOmZpbGVzdG9yZTpmaWxlOjljZDBkYThlLThkNDAtNGQ2My04YTVhLTFlZDAxZGUzZjVhMSI6WyJyZWFkIl19LCJuYmYiOjE1MDI4OTA2NDEsImV4cCI6MTUwMjg5MTA2MX0.SEQIXy7OBXGrx0Ruh9YgVAYUMo-042_-RNj6X-McMhs&client=514f1f5d-6bdf-46bc-bead-a16851671fa7
adding changesets
adding manifests
adding file changes

I imagine we could fix this by either fixing the directions to use the latest version of Eigen in the first place (which might cause bugs for people when the latest version of Eigen breaks), or modifying setup.py to use the version specified in the manual installation instructions. But it might be worth considering using a release version of Eigen like this commit, which would solve the problem and also remove the dependency on mercurial: #608

What do you think?

[danielhers]

It just checks if the eigen directory exists, and if so, skips cloning it, so it doesn't matter what version it is. How did you specify the EIGEN3_INCLUDE_DIR to setup.py?

[neubig]

I didn't. I just followed the manual installation directions exactly.

[danielhers]

So it has no way to know that you are using a non-standard eigen path...

[danielhers]

By the way, installing with CUDA should also be possible now by giving setup.py the same environment variables as you would give cmake. I guess this and the eigen path thing should be in the documentation.

[neubig]

Quoted from my previous post: I tried the manual install instructions again in a clean environment and followed the instructions word-for-word.

To re-phrase: I stopped using a custom install path, and ran the entire manual install instructions, which include cloning Eigen to a local directory. If necessary, I can create a log with all the commands I ran and the result. Would this be useful?

[danielhers]

I see now, sorry I misunderstood before. Seems like the build directory when using the command I wrote in the instructions isn't what it's supposed to be. I'll check why.

[neubig]

Thanks! I'm also trying to create a log file that shows what happens just in case this is an environment specific thing.

And yes, I think good documentation about how to install with CUDA and how to specify environmental variables would be very useful for the average user.

[danielhers]

I haven't tested the CUDA installation with pip yet, it's just in theory that it should work. I'll update the docs after testing it.

[danielhers]

Seems like it was cloning eigen again since it really didn't find it inside dynet/build/eigen, as the instructions say to clone it alongside the root dynet directory. And then perhaps the compilation happened again because some dependencies changed, such as eigen. Anyway, I added a --skip-build flag to force skipping the build to avoid this. --build-dir is still necessary to tell it where to find the Python files, though.

[danielhers]

@neubig Fixed, now it installs only once. And I enabled testing for pip on OSX as a bonus :)

[neubig]

Great, thanks! I'm still having some trouble though... Perhaps this is resulting from a slightly old build environment using cmake? Here are three logs.

1. test-dynet-relative.txt: the default settings according to the instructions EIGEN3_INCLUDE_DIR=../../eigen. This breaks, because it uses the relative path from the directory referenced by cmake .. which is the top DyNet directory.
2. test-dynet-relative-singlestep.txt: this fixes the relative path to ../eigen, and passes C++ build, but fails on Python.
3. test-dynet-absolute.txt: this uses the absolute path $HOME/work/dynet-base/eigen, but similarly fails on Python.

I'm out of time to debug now, but if you have any ideas I may be able to try them later today or tonight. One idea I had was to read EIGEN3_INCLUDE_DIR from CMakeCache.txt in the build directory.

test-dynet-absolute.txt
test-dynet-relative.txt
test-dynet-singlestep-relative.txt

[danielhers]

It's not enough to give it to cmake - you need to give it to setup.py too, otherwise it looks at its default location (build/eigen). I'll update the docs.. It could be determined automatically but I think it might just complicate things.

[danielhers]

I hope it's better now. Sorry you had to go through the trouble of debugging this.

[neubig]

Great, it's working for me now. Will merge.