Skip to content

Minimal MSVC compatibility adjustments#2084

Closed
classner wants to merge 101 commits intoBVLC:masterfrom
classner:master
Closed

Minimal MSVC compatibility adjustments#2084
classner wants to merge 101 commits intoBVLC:masterfrom
classner:master

Conversation

@classner
Copy link

@classner classner commented Mar 9, 2015

Dear caffe-team,

since our lab infrastructure is mostly running on Windows, I looked for a way to use caffe on this platform. I am well aware of Niu Zhihengs fork, but wanted to have the most recent version of caffe available.

Since I well-understand that you might not want your core code and build system cluttered with platform specifics, I decided to write a non-intrusive minimal build platform for caffe, the caffe-brewer ;) (https://github.com/ChrislS/caffe-brewer). Since I know my way around the Scons build system best and it's easily possible to script builds of many subprojects with SCons, I decided to use that one.

The caffe-brewer git project pulls in the caffe project as submodule as well as as many dependencies as possible and builds caffe on Linux and Windows out of the box.

With this pull request, I just suggest to integrate a minimal set of changes to make the core caffe code compile on Windows. In case you are willing to integrate them, I can point the caffe-brewer to the original BVLC repository, which would be great!

@classner
Copy link
Author

classner commented Mar 9, 2015

The first Travis CI build failed, because -std=c++0x was missing on the build files. I used the chrono features to replace the Linux-only timing commands with the standardized platform independent ones.

@Geekrick88
Copy link

Dear Chris,
Thanks for the additions. As I am also trying to integrate caffe with an application that requires the Kinect v2, I have no other option other than windows. I have a version of the current master branch incorporating these changes and a few more. The only problem I am facing now is building a DLL instead of a lib, because static linking strips off the symbols that are registered globally as they are not referenced directly in the caffe.exe or other binaries. This includes the layers and other stuff. There is an ugly hack where the code of the layer register class has to be copied to the binaries for them to build. This can be alleviated by dynamic linking. Since visual studio does not export symbols by default, a small macro defining the dllexport/dllimport has to be incorporated. For most of the part all the templated classes in caffe use explicit instantiation to float and double (sometimes uint and int as well) using a macro, so does the GPU functions. Hence just by changing the macros as

#define INSTANTIATE_CLASS(classname)
char gInstantiationGuard##classname;
template Caffe_EXPORT class classname;
template Caffe_EXPORT class classname;

where Caffe_EXPORT is defined as follows

#if !defined WIN32
#define Caffe_EXPORT
#elif defined caffe_EXPORTS
#define Caffe_EXPORT __declspec(dllexport)
#else
#define Caffe_EXPORT __declspec(dllimport)
#endif

This solved the problems with the linker errors with the Classes. However if I do the same with the GPU functions, it is not compiling and complaining about function specified with dllimport cannot be defined. If I remove them the GPU functions are not included as exported symbols and genertae linker error when trying to link to the caffe library. Any idea how to solve the problem?

@classner
Copy link
Author

Hi Rick!
The global objects for registering the layers really make static linking on Windows impossable to my knowledge. Your modification of the INSTANTIATE_CLASS macro seems to be a smart hack! For the GPU functions, things might get more difficult, but I'm not a CUDA expert, so I unfortunately can't give you good hints there.

My suggested solution to the problem is different. I wanted to create a possibility to work with CAFFE on Windows with the cutting edge version, but without requiring the DEV team to incorporate large changes to their build system. So I created this minimal patch of chages to the source, that might be acceptable and integrated into mainstream. The caffe-brewer project I linked before then builds all object files (CUDA and non-CUDA) and a set of .lib files. You can then use the CAFFE functions platform independently, by integrating the object files with your own ones, and linking against the rest of the libraries.

Even though this is a little tedious, it seemed to be the smartest way for me.

Btw., it seems that the issue is not marked "ready for review", because the first CI build failed. Do you know how to retrigger a build?

@Geekrick88
Copy link

It seems you need to close this PR and resubmit. It will re-trigger the travis. It is still showing merge conflicts so you may need to rebase it again against the master. Then we can post the comments again to continue the discussion.

@Geekrick88
Copy link

I also think that the definitions like snprintf and other windows specific substitutions like _getpid and all should be put in a single header file. I named it Win32Port.hpp. Then it can be included in the common.hpp with #ifdef WIN32 guard. You may need to include it again in the tools if they do not include the caffe common.hpp. Then it becomes even more non invasive. Also in the BNLL layer the threshold can remain as a variable instred of a MACRO by constant float kBNLL_THRESHOLD = 50.; The constant qualifier is the correct way for CUDA device code constant. One case where NVCC on MSVC strictly adheres to the standard but the NVCC on GCC and LLVM implementations are lax.

@shelhamer
Copy link
Member

Don't close and open -- rebase and push to your fork. Since this isn't s
Linux build it can't pass our linux Travis but if other Windows users /
devs can comment that will be helpful.

Thanks for working on cross platform porting.
On Sun, Mar 15, 2015 at 11:17 Rick notifications@github.com wrote:

I also think that the definitions like snprintf and other windows specific
substitutions like _getpid and all should be put in a single header file. I
named it Win32Port.hpp. Then it can be included in the common.hpp with
#ifdef WIN32 guard. You may need to include it again in the tools if they
do not include the caffe common.hpp. Then it becomes even more non
invasive.


Reply to this email directly or view it on GitHub
#2084 (comment).

Christoph Lassner and others added 12 commits March 16, 2015 15:14
set the right rpath for tools and examples respectively

thanks for the report @mees!
… was overwritten with symlink created at build time and installed with install(DIRECTORY ...)
… systems).

This commit specifies Python2 with which cpp_lint.py works :-)
Commands, such as $(error ...), are not allowed to be indented with tabs
outside of targets, throwing an error instead of outputting the actual
error. The solution is to use innocuous spaces instead. Ideally, spaces
should be used everywhere outside targets, but since make does not mind
it if variable assignments are tab-indented outside targets, a complete
overhaul is not necessary. However, if more errors are added, it might
make more sense to be consistent.

Also, make will already add a period so I removed it.
longjon and others added 26 commits March 16, 2015 15:20
...and fix up detector quote convention.
- download CaffeNet if it isn't there
- switch to caffe.Net
- reshape net for single input
- explain param, bias indexing
- update output for N-D blobs
- N-D blob parameter dimensions
- add filtering section to net surgery example
- make Gaussian blur and Sobel edge kernels
- alter biases
- do flat assignment instead of reshape and explain memory layout
- make dir for surgery files
- add a little explanation
- solve from Python and the command line
- time scikit-learn and caffe
- fix test iterations (number of instances / batch size)
call forward to warm-start the demo so the first request isn't slow.
See BVLC model license details on the model zoo page.

[Re-commit of 6b84206 which somehow went missing.]
described in Kaiming He et al, "Delving Deep into Rectifiers: Surpassing
Human-Level Performance on ImageNet Classification", arxiv 2015.

Belows are commit message histories that I had while developing.

PReLULayer takes FillerParameter for init

PReLU testing consistency with ReLU

Fix : PReLU test concistency check

PReLU tests in-place computation, and it failed in GPU

Fix: PReLU in-place backward in GPU

PReLULayer called an incorrect API for copying
data (caffe_gpu_memcpy). First argment of `caffe_gpu_memcpy` should be
size of memory region in byte. I modified to use `caffe_copy` function.

Fix: style errors

Fix: number of axes of input blob must be >= 2

Use 1D blob, zero-D blob.

Rename: hw -> dim
Use ReadNetParamsFromBinaryFileOrDie to read a net param when restoring
from a saved solverstate, which upgrades old nets, rather than
ReadProtoFromBinaryFile.
@classner
Copy link
Author

So, this one is messed up. I'm perfectly familiar with git for the more standard tasks, but didn't use rebase before and did it obviously wrong. Nevermind, I'll collect the changes in a minimal header and resubmit an according pull request. The build will run perfectly fine, since I hardly changed anything with this commit and just added very few #ifdef pragmas.

The entire platform independent build system is in the (non-invasive) caffe-brewer project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Comments