Skip to content
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

CuDNN 7 cases added, warnings removed #5869

Closed

Conversation

gineshidalgo99
Copy link
Contributor

@gineshidalgo99 gineshidalgo99 commented Aug 23, 2017

Added some cuDNN 7 support.

Now it works without any warnings in both cuDNN 7 and cuDNN < 7.

Otherwise, it pop ups the following warnings in cuDNN 7 (I'm enabling all warnings):

In file included from ./include/caffe/util/device_alternate.hpp:40:0,
                 from ./include/caffe/common.hpp:19,
                 from ./include/caffe/blob.hpp:8,
                 from ./include/caffe/layers/loss_layer.hpp:6,
                 from src/caffe/layers/loss_layer.cpp:3:
./include/caffe/util/cudnn.hpp: In function ‘const char* cudnnGetErrorString(cudnnStatus_t)’:
./include/caffe/util/cudnn.hpp:21:10: warning: enumeration value ‘CUDNN_STATUS_RUNTIME_IN_PROGRESS’ not handled in switch [-Wswitch]
   switch (status) {
          ^
./include/caffe/util/cudnn.hpp:21:10: warning: enumeration value ‘CUDNN_STATUS_RUNTIME_FP_OVERFLOW’ not handled in switch [-Wswitch]
CXX src/caffe/layers/absval_layer.cpp
In file included from ./include/caffe/util/device_alternate.hpp:40:0,
                 from ./include/caffe/common.hpp:19,
                 from ./include/caffe/blob.hpp:8,
                 from ./include/caffe/layer.hpp:8,
                 from ./include/caffe/layers/parameter_layer.hpp:6,
                 from src/caffe/layers/parameter_layer.cpp:1:

The code difference is adding these cases:

#if CUDNN_VERSION_MIN(7, 0, 0)
    case CUDNN_STATUS_RUNTIME_IN_PROGRESS:
      return "CUDNN_STATUS_RUNTIME_IN_PROGRESS";
    case CUDNN_STATUS_RUNTIME_FP_OVERFLOW:
      return "CUDNN_STATUS_RUNTIME_FP_OVERFLOW";
#endif

Highly tested and it's being currently used in OpenPose

@gineshidalgo99
Copy link
Contributor Author

Travis builds worked nicely. I only added the case for cuDNN > 7. And I used #if #endif in order to avoid crashing in cuDNN < 7 (tested in 5.1, 6 and 7)

@gineshidalgo99
Copy link
Contributor Author

Side note: I've been using it for the last 2 weeks (and I have 2 servers, one with cuDNN 7 and one with 5.1), and the same code works nicely in both versions.

@Noiredd
Copy link
Member

Noiredd commented Nov 2, 2017

@shelhamer We've got 3 PRs for cuDNN 7 support: this one (which was first), #5879 (which also adds grouped convolution) and #5972 (which also updates installation instructions and Travis dependencies). Personally, I think the combination of the latter two would be best - what do you think?

@Noiredd
Copy link
Member

Noiredd commented Nov 7, 2017

I decided to merge #5972 instead, as it also changes Travis to test with cuDNN 7. Offering any support for the new version is too important to keep us waiting for review of #5879.

Still, thank you for your contribution @gineshidalgo99!

@Noiredd Noiredd closed this Nov 7, 2017
@cheyuanxiaozi
Copy link

when using cudnn 7 , the variables in 'cudnn.h' are not defined in all of cudnn layers. Is there anyone see this error?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants