odlgroup / odlcuda

C++ backend for ODL
GNU General Public License v3.0
5 stars 2 forks source link

Make plugin #16

Closed adler-j closed 8 years ago

adler-j commented 8 years ago

Work in progress.

Changes makes this a plugin instead of a subpackage.

kohr-h commented 8 years ago

Looking good up to minor things. I'll try to build and install myself first though.

kohr-h commented 8 years ago

I didn't look too closely on the doc since I'm anyway doing a big overhaul. Design-wise it looks good.

kohr-h commented 8 years ago

Okay, after some playing around and observing that conda tries to be smart and not to copy stuff around more than it has to, I finally got a freshly built odlpp installed. When trying to import it, I get

File "/home/hkohr/miniconda3/envs/odlpp_test_27/lib/python2.7/site-packages/odlpp/__init__.py", line 23, in <module>
    from . import odlpp_cuda
ImportError: cannot import name odlpp_cuda

Looking at the output from the build process, it's no surprise:

...
[100%] Linking CXX shared library ../../bin/libodlpp_cuda.so
[100%] Built target odlpp_cuda
Scanning dependencies of target pyinstall
running install
running bdist_egg
running egg_info
creating odlpp.egg-info
writing requirements to odlpp.egg-info/requires.txt
writing odlpp.egg-info/PKG-INFO
writing top-level names to odlpp.egg-info/top_level.txt
writing dependency_links to odlpp.egg-info/dependency_links.txt
writing entry points to odlpp.egg-info/entry_points.txt
writing manifest file 'odlpp.egg-info/SOURCES.txt'
reading manifest file 'odlpp.egg-info/SOURCES.txt'
writing manifest file 'odlpp.egg-info/SOURCES.txt'
installing library code to build/bdist.linux-x86_64/egg
running install_lib
running build_py
creating build
creating build/lib
creating build/lib/odlpp
copying ./odl_pluggin.py -> build/lib/odlpp
copying ./cu_ntuples.py -> build/lib/odlpp
copying ./__init__.py -> build/lib/odlpp
copying ./ufuncs.py -> build/lib/odlpp
creating build/bdist.linux-x86_64
creating build/bdist.linux-x86_64/egg
creating build/bdist.linux-x86_64/egg/odlpp
copying build/lib/odlpp/odl_pluggin.py -> build/bdist.linux-x86_64/egg/odlpp
copying build/lib/odlpp/cu_ntuples.py -> build/bdist.linux-x86_64/egg/odlpp
copying build/lib/odlpp/__init__.py -> build/bdist.linux-x86_64/egg/odlpp
copying build/lib/odlpp/ufuncs.py -> build/bdist.linux-x86_64/egg/odlpp
byte-compiling build/bdist.linux-x86_64/egg/odlpp/odl_pluggin.py to odl_pluggin.pyc
byte-compiling build/bdist.linux-x86_64/egg/odlpp/cu_ntuples.py to cu_ntuples.pyc
byte-compiling build/bdist.linux-x86_64/egg/odlpp/__init__.py to __init__.pyc
byte-compiling build/bdist.linux-x86_64/egg/odlpp/ufuncs.py to ufuncs.pyc
creating build/bdist.linux-x86_64/egg/EGG-INFO
copying odlpp.egg-info/PKG-INFO -> build/bdist.linux-x86_64/egg/EGG-INFO
copying odlpp.egg-info/SOURCES.txt -> build/bdist.linux-x86_64/egg/EGG-INFO
copying odlpp.egg-info/dependency_links.txt -> build/bdist.linux-x86_64/egg/EGG-INFO
copying odlpp.egg-info/entry_points.txt -> build/bdist.linux-x86_64/egg/EGG-INFO
copying odlpp.egg-info/requires.txt -> build/bdist.linux-x86_64/egg/EGG-INFO
copying odlpp.egg-info/top_level.txt -> build/bdist.linux-x86_64/egg/EGG-INFO
zip_safe flag not set; analyzing archive contents...
odlpp.__init__: module references __file__
creating dist
creating 'dist/odlpp-0.2.0-py2.7.egg' and adding 'build/bdist.linux-x86_64/egg' to it
removing 'build/bdist.linux-x86_64/egg' (and everything under it)
Processing odlpp-0.2.0-py2.7.egg
creating /home/hkohr/miniconda3/envs/_build/lib/python2.7/site-packages/odlpp-0.2.0-py2.7.egg
Extracting odlpp-0.2.0-py2.7.egg to /home/hkohr/miniconda3/envs/_build/lib/python2.7/site-packages
Adding odlpp 0.2.0 to easy-install.pth file

Installed /home/hkohr/miniconda3/envs/_build/lib/python2.7/site-packages/odlpp-0.2.0-py2.7.egg

The compiled stuff is never touched and does not end up in the final package. Thus, only a skeleton package is installed. I'm not quite sure why it is like that. Possible explanations are (1) the CMake commands never copy files or (2) conda wraps up the package before the libraries are copied or ignores them in some other way. I can build the code by hand to exclude reason (1). If it's related to (2), then probably we really need a setup.py based build so conda will see all the output, or some other way to manually copy the compiled files.

kohr-h commented 8 years ago

Same output for building "by hand", library files are not copied.

adler-j commented 8 years ago

Absolutely no idea why it would not do that, it does the copy on windows but I had some issues with it (see the __path__ thing). Could you try checking the package_data and see if you can get it to work on linux?

kohr-h commented 8 years ago

If I replace the package_data in odlpp/setup.py by package_data={'' : ['*.pyd', '*.so']}, the files are included, and things finally work. Testing the conda build now.

adler-j commented 8 years ago

¯_(ツ)_/¯

Great!

kohr-h commented 8 years ago

I think the proper way of doing this is to create a MANIFEST.in file (see here) that contains all files apart from Python scripts/modules that are to be included in the distribution.

kohr-h commented 8 years ago

Added a MANIFEST.in and it is actually used during the setup.py install command. However, it does not find the README.md and LICENSE files since they are not copied to the build directory. Here are the contents of build/bin where python setup.py install is run:

build          dist         libodlcpputils_cuda.a    libodlcpputils_test.a  MANIFEST.in     odlpp_cuda.so   setup.py
cu_ntuples.py  __init__.py  libodlcpputils_python.a  libodlpp_cuda.so       odl_pluggin.py  odlpp.egg-info  ufuncs.py

I know too little about CMake to figure out why it behaves like this. Perhaps the .in suffix makes it believe it's a config file or similar and copies it by default. Stupid, I created the file in build/bin and copied it over. Some way to make sure that also the other all metadata files are copied would be good.

kohr-h commented 8 years ago

Some way to make sure that also the other all metadata files are copied would be good.

Ping. Shouldn't be a big thing and would make this a working package.

TODOs:

EDIT: added main TODO to TODO list.

adler-j commented 8 years ago

I'll try to get this done once I get back to KTH after midsummer, agree that it should be prio and we need to get it in rather soon. It would also enable the conda branch.

The checklist is very nicely compiled and should cover most things.

Regarding conda however, having a single branch for linux, getting that merged (which seems close?) and then making a new branch for windows would be totally fine.

kohr-h commented 8 years ago

Regarding conda however, having a single branch for linux, getting that merged (which seems close?) and then making a new branch for windows would be totally fine.

Sure.

adler-j commented 8 years ago

Any chance you can push your changes to the branch so I can keep working from there?

kohr-h commented 8 years ago

If you don't need conda stuff, simply cherry-pick 71db46cd. It contains the MANIFEST.in file. No further relevant changes.