Install with pip

[psheehan]

Don't know if this is of interest to you or if it messes up your conda install setup, but I was able to put together a setup.py file that allows you to use pip to install (I don't use conda and got tired of building by hand each time I updated python :) ). It probably doesn't account for all of the possible cmake options, but any additional relevant ones should be fairly easy to build in.

The only slight catch is that because the cmake install puts libgalario.dylib and libgalario_single.dylib into the --prefix/lib directory, that directory needs to be in the LD_LIBRARY_PATH for the python installation to find it on import. This is probably fine for most standard installations of python... but can cause issues for odd ones, especially on Macs as it seems like they no longer let you actually set LD_LIBRARY_PATH. Not sure how to get around this, though, without more major revisions.

Anyways, feel free to take if you are interested, or to discard if not!

[codecov[bot]]

Codecov Report

Merging #182 (0b92b3e) into master (f5d11f8) will not change coverage. The diff coverage is n/a.

:exclamation: Current head 0b92b3e differs from pull request most recent head 0f3d218. Consider uploading reports for the commit 0f3d218 to get more accurate results

@@           Coverage Diff           @@
##           master     #182   +/-   ##
=======================================
  Coverage   97.98%   97.98%           
=======================================
  Files           5        5           
  Lines         496      496           
=======================================
  Hits          486      486           
  Misses         10       10           

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update f5d11f8...0f3d218. Read the comment docs.

[mtazzari]

Thank you very much Patrick for this very nice addition! Thanks for taking the time for posting the code! I'll have a look as soon as I have a moment. In principle I'd be very happy to build galario with pip too, as an additional way to get galario on top of conda or manual installation with cmake. When we first released galario we opted for conda + cmake which gave us the most cross-platform solution (as you noted, there might be some difficulties in getting pip to work when custom installations have a non-default LD_LIBRARY_PATH value). Indeed, compared to pip, conda allowed us to pin specific versions of fftw and openmp libraries so that galario always has the desired performance. Also, cmake makes it easy to build the GPU version. If I remember correctly, pip was not giving us the same capability of conda. Anyway, I'll review this issue again with @fredRos!

PS Does this implementation with pip build the GPU version too?

[psheehan]

Hey Marco, of course, glad to share, especially if it'll be useful!

pip doesn't quite provide the same level of end-to-end, all included compiling environment, it just handles the Python side of things, so anyone installing this way would need to make sure they have cmake and fftw installed on their own (I think OpenMP it can handle as long as the compiler used supports it). This is basically just a fancy way to do the "cmake", "make" and "make install" steps and have things get put in reasonable places that are recorded for easy removal. But might save some folks the trouble of having to actually do/configure those steps properly (not that they are hard, but "pip install galario" does save a few steps).

Right now it doesn't build the GPU version because I hard coded -DGALARIO_CHECK_CUDA=0, but I think if you were to remove that line then it should be able to compile with CUDA. There's no reason I can think of for why that wouldn't work... There might be another minor update or two to make sure that the files produced by compiling with CUDA end up in the right place, but that wouldn't be hard. I do have a linuxbox now, so I can give that a try and see how it works! I'm overall less familiar with CUDA, so might need a bit of help with that.

But yeah, if interested please do use, and I'm happy to make some updates to get it working how you'd like!

[psheehan]

Just a quick update, since I had some inspiration in the last week when I got a new computer. Installing with pip should now no longer have any issues with non-standard LD_LIBRARY_PATH's (I set the RPATH explicitly to include /lib), and I enabled building with CUDA and it all seems to work!

[mtazzari]

Wow, thank you Patrick! This is really great! Sorry for the delay in picking this up - great job & thank you for contributing with a PR! I'm starting to test this...will take a while on my side but I will follow up.

[mtazzari]

For some reason the tests on GH Actions are run on the forked repo (psheehan/galario) and not on mtazzari/galario. I believe this is not right. Looking into this, there's a thread: https://github.com/WordPress/gutenberg/issues/17324

EDIT: the behaviour is actually right for security reasons. See https://github.blog/2020-08-03-github-actions-improvements-for-fork-and-pull-request-workflows/

[mtazzari]

Hi Patrick, I'm trying to fix the issue (getting the tests to pass on mtazzari/galario rather than on psheehan/galario). It would help me if you could allow me to push to your psheehan/galario:master from which you have opened the PR. This is how you do it: https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/allowing-changes-to-a-pull-request-branch-created-from-a-fork

Let me know if it's ok.

[psheehan]

Hey Marco, this is ok by me! I took a look at the instructions you sent (https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/allowing-changes-to-a-pull-request-branch-created-from-a-fork) and when I came back to this page, it seemed like the box that that page mentioned was already checked here. For good measure I unchecked and checked it again - does that let you do what you need to do?

[mtazzari]

Thank you Patrick for checking that! I'll try again then!

[mtazzari]

Hi Patrick, I've seen these latest commits. Thanks for contributing - much appreciated! I'll try to review these as soon as I have some time, hopefully in the next week or so.

[psheehan]

Hi Marco, not a problem, and no hurry! Also on my list is to send some more concrete details of what I've done. In short, I've made a pip-install build system more robust. It also comes with the possibility (necessity?) of reworking the conda build system to cover the conda and pip builds with the same code. But there would be some pros/cons of doing that that I should probably detail, along with the current status - there might be a few more things to be tackled before something like this could/should be merged.