manodeep / Corrfunc

⚡️⚡️⚡️Blazing fast correlation functions on the CPU.
https://corrfunc.readthedocs.io
MIT License
165 stars 50 forks source link

Building Corrfunc with conda-build #129

Closed nickhand closed 7 years ago

nickhand commented 7 years ago

We (@rainwoodman and I) would like to use Corrfunc for MPI-enabled correlation function calculations in nbodykit (see bccp/nbodykit#355, for a start). We prefer to have all of the dependencies available on conda, either through the defaults channel or our own bccp channel.

We've built Corrfunc on Linux no problem but are having some issues on Mac when conda-build tries to set the linking paths properly. Have you built on Mac with conda-build before? The issue we are seeing is here:

https://travis-ci.org/bccp/conda-channel-bccp/jobs/247771299#L1451

otool -L spits out that the countpairs_mocks.so object is linked against countpairs_mocks, which is creating issues because that isn't a valid file name and conda-build can't find it.

manodeep commented 7 years ago

Thanks for reporting the issue. First of all, we would really love to have a conda install option for Corrfunc and all my previous attempts have failed for OSX. I do not know how to fix it but I am happy to help in any to diagnose and find a solution.

What I recall was that the _countpairs_mocks.so needed to be copied into a different path. Ultimately, this might be caused by the fact that I have tried to do all the installation via make even when python setup.py has been called.

What do you mean that _countpairs_mocks.so is not a valid filename? I am not sure I understood that -- is there some naming convention that you are referring to?

rainwoodman commented 7 years ago

Is it possible to get rid of make? We had pretty good experience with that when wrapping class and other packages.

manodeep commented 7 years ago

@rainwoodman Unlikely to get rid of make. Currently python isn't a dependency for Corrfunc and I would like to keep it that way.

There are future plans to create a separate and more generic python-only package. But that's a different story :)

rainwoodman commented 7 years ago

I mean setup.py doesn't necessarily need to call make if you are willing to flatten the directory structure a bit. Some of my similar packages follow this approach. e.g. bigfile has cmake, make and setup.py:

https://github.com/rainwoodman/bigfile

nickhand commented 7 years ago

I got it to build with conda on Mac by renaming the install_name to include the .so extension. See e957ce52. I don't think this breaks anything else.

rainwoodman commented 7 years ago

I looked at the directory structure. I see two possibilities:

rainwoodman commented 7 years ago

I still think a single .so file is easier to access than many .so files for the long run. That being said, since your minimal fix works I won't pursue this further.

manodeep commented 7 years ago

@rainwoodman Thanks again for the suggestions. Under the current scheme, every Corrfunc API creates a static library (i.e., files are always compiled with the -fPIC option). These static libraries then linked to produce two shared libraries (the python extensions) -- one for theory routines and one for mocks. Keeping the python routines in separate shared libraries helps maintain different mental contexts for the mocks and theory routines.

@nickhand Would it be possible to update the meta.yaml file and/or send a PR?

Thanks so much to both of you for checking up on this.

nickhand commented 7 years ago

@manodeep yep, I'll put it in a PR

manodeep commented 7 years ago

See PR #131.

@nickhand @rainwoodman Are you guys okay with closing this issue?

nickhand commented 7 years ago

sounds good to me!