-
Notifications
You must be signed in to change notification settings - Fork 15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Install with pip #182
base: master
Are you sure you want to change the base?
Install with pip #182
Conversation
Codecov Report
@@ 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.
|
Thank you very much Patrick for this very nice addition! Thanks for taking the time for posting the code! PS Does this implementation with |
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! |
… as that is where they are installed to.
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! |
Wow, thank you Patrick! This is really great! |
For some reason the tests on GH Actions are run on the forked repo (psheehan/galario) and not on mtazzari/galario. EDIT: the behaviour is actually right for security reasons. |
Hi Patrick, I'm trying to fix the issue (getting the tests to pass on mtazzari/galario rather than on psheehan/galario). Let me know if it's ok. |
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? |
Thank you Patrick for checking that! I'll try again then! |
…ng sphinx-build instead of cmake.
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. |
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. |
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!