dnouri / cuda-convnet

My fork of Alex Krizhevsky's cuda-convnet from 2013 where I added dropout, among other features.
http://code.google.com/p/cuda-convnet/
253 stars 147 forks source link

Added support for CMake #4

Closed kashif closed 10 years ago

kashif commented 10 years ago

This adds support for CMake and I also cleaned up some whitespaces and also fixed the Readme.

Now this will generate a convnet.so shared library.

Tested on OSX.

And here is the diff with the whitespace changes ignored:

https://github.com/dnouri/cuda-convnet/pull/4/files?w=1

dnouri commented 10 years ago

Hi Kashif, thanks for your pull request. I guess the motivation behind using Cmake is that one can get rid of build.sh and friends and make the build process more flexible. Correct?

Would you explain the motivation for adding numpy headers in the source tree? Do you consider it too hard to obtain them? I don't remember having had trouble with those.

I'm seeing some changes related to CUDA 6.0. Do we only support CUDA 6.0 now? The Readme changes only remove information about which CUDA to use. Would you sum up and maybe add to the Readme a list of platforms and CUDA versions that you tested your changes against? Thanks.

kashif commented 10 years ago

To answer you the motivation for cmake was to be able to work on linux mac and windows... still have to test on windows :smile:

The numpy headers are gone... I was having an issue trying to get cmake to find them, but I figured it out.

At the moment i think we can go with cuda 5+ support. This as far as I remember is not in this pull request. After this pull request is merged, I will like to send you those changes, were I moved to cublas_v2 for example and used curand etc. But yes I can do that in the next pull request. Hope this helps?

Thanks!

dnouri commented 10 years ago

It looks like I got confused about the numpy headers. The huge amount of whitespace changes presumably to make your linter happy makes it really hard to look at the diff; but I'll deal with it.

In CMakeLists.txt I see a reference to "/Developer/NVIDIA/CUDA-6.0". With which versions of CUDA and on which platforms did you test your changes? Let's add that information to the README.

kashif commented 10 years ago

Ah yes the "/Developer/NVIDIA/CUDA-6.0" is just a hint, just in case it doesnt find stuff in the normal places... Yes so to see the diff without whitespace go to:

https://github.com/dnouri/cuda-convnet/pull/4/files?w=1

dnouri commented 10 years ago

Please also follow up on my other remarks: With which versions of CUDA and on which platforms did you test your changes? Let's add that information to the README.

The ?w=1 thing is useful, I'll remember that.

dnouri commented 10 years ago

Thanks, merged your changes. Being confused by the many commits, I also assumed that API changes and a CUDA update were part of this. It looks like that wasn't the case and thus I assume requirements haven't changed from before.

It'll be nice to get exact requirements when we integrate those other changes that you talked about.

invisibleroads commented 10 years ago

Hi kashif, do you know what this error means? This error is new as of your recent cmake changes.

>> import convnet
ImportError: libcommon.so: cannot open shared object file: No such file or directory

Previously, I was successfully running dnouri's version at https://github.com/dnouri/cuda-convnet/commit/0d9e86bc2ed6401249481a8da61bcbe1a6cdeec8 on Fedora 20 64-bit.

kashif commented 10 years ago

Dear @invisibleroads

Yes I see, its caused by the fact that the LD_LIBRARY_PATH cannot find it when it loads convnet.so. Can you kindly try to remove the set (BUILD_SHARED_LIBS ON) and try?

Thanks!

invisibleroads commented 10 years ago

Resolved in https://github.com/dnouri/cuda-convnet/pull/8