autumnai / collenchyma

Extendable HPC-Framework for CUDA, OpenCL and common CPU
http://autumnai.github.io/collenchyma
Apache License 2.0
475 stars 33 forks source link

chore/osx: support building on os x and test this on travis #59

Closed reem closed 8 years ago

reem commented 8 years ago

The newly-added build script adds the default installation location of cuda to rustc's library search path.

Fixes #46

reem commented 8 years ago

I actually don't think this fixes any issues re: linking OpenCl, but permanently fixing/testing that is blocked on #16.

reem commented 8 years ago

Just to expand a little: the travis configuration actually does not test with either the opencl or cuda features, because of #16 and because travis doesn't have nvidia GPU nodes respectively, but this fix works on my local machine (OS X 11) with a standard cuda install, and travis now just tests that collenchyma compiles on os x.

MichaelHirn commented 8 years ago

As already mentioned in #46 at this comment I would like to extend the PR a bit more and introduce already the basic design of the proposed solution in #46. I would make a PR to your branch if you are up to that?

That's correct, that we can't really test runtime behaviour with Travis for GPUs (CUDA, OpenCL) and because of the Travis bug, as described in more detail at #16, also no OpenCL behaviour for now. And there seems to be not CI service who provides GPU support, yet. But if we can test compilement on multiple platforms, that is already incredibly helpful.

Thanks for the .travis.yml :+1:

reem commented 8 years ago

I would definitely take a PR! Unfortunately it looks like the nvidia cuda installers don't support installing to a specific directory, meaning it appears our only option would be to install cuda globally on the users system if it is not already there.

Personally I don't think that's a very appealing route (I'd be quite surprised if cargo install uses-leaf would globally install software), and it would be better to just detect the lack of cuda (or other libraries) and bail out with an error message including a link to instructions on how to get up and running (on os x this is as easy as brew cask install cuda). This is effectively the approach taken by other links-to-C crates like rust-openssl.

As a workaround for the lack of GPU support on travis, we could set up an aws account, include the credentials as encrypted env vars, and run a cuda test on an aws gpu node. This would allow continuous testing of cuda (at least on linux, I don't think you can run os x on an aws gpu node) at the cost of a steep increase in CI setup complexity.

reem commented 8 years ago

I've updated this PR to only include the changes to the travis configuration, we can deal with adding cuda and opencl support/continuous testing in subsequent PRs.

MichaelHirn commented 8 years ago

Agreed, installing CUDA globally is something that we should not do when building Collenchyma. A few days ago, we received from NVIDIA direct download links for their cuDNN library, so people can use collenchyma-nn (Leaf on NVIDIA GPUs) more easily. I assume they would have that for CUDA as well. I will ask him.

But for now, I will merge this PR and we introduce the build support in a subsequent one. When you alter the commit message to reflect the new code changes, I think we are ready to merge.

reem commented 8 years ago

What would you want to change in the commit message? Not fix #46? I think we should close #46 and open new issues for support/testing of opencl and cuda on os x respectively, but I can also just remove the fixes piece and leave it open.

MichaelHirn commented 8 years ago

I assumed you are altering the commit (removing the build.rs file again) as to just introduce OS X support on Travis with this PR because of

I've updated this PR to only include the changes to the Travis configuration, we can deal with adding cuda and opencl support/continuous testing in subsequent PRs

If we do that, then I would propose to adapt the commit message from

chore/osx: support building on os x and test this on travis

Fixes: #46

to something like

chore/osx: add os x support on travis

I apologize for being not clear enough previously.

Then regarding the #46 issue, I am would do it like you proposed.

Maybe just opening one issue, which bundles the build fixes for opencl/cuda and the various platforms and links to the #46. As soon as we have the osx platform supported we can close #46.

MichaelHirn commented 8 years ago

Great. Let's merge.

MichaelHirn commented 8 years ago

@homu r+

homu commented 8 years ago

:pushpin: Commit 2ac7e87 has been approved by MichaelHirn

homu commented 8 years ago

:zap: Test exempted - status