-
Notifications
You must be signed in to change notification settings - Fork 18.6k
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
Basic cuDNN v3 support (update) #3160
Conversation
@shelhamer Regarding the test failure for Groups -- it seems like this->weight_offset_ for the cuDNN routines is getting set incorrectly / not set (either way, it's wrong!) This seems to have been introduced in 9d8206e Setting it back explicitly to: in CuDNNConvolutionLayer::Setup seems to fix the issue, not sure if there's a better way - let me know and I'll update the PR |
Great 👍 |
cuDNN v3 is not itself backward compatible with v2, so adopting v3 in this PR does deprecate v2. We plan to follow the latest cuDNN version in master but keep compatability as the cuDNN interface itself allows. |
} | ||
Dtype* bias_diff = NULL; | ||
if (this->bias_term_ && this->param_propagate_down_[1]) { | ||
bias_diff = this->blobs_[1]->mutable_gpu_diff(); | ||
caffe_gpu_set(this->blobs_[1]->count(), Dtype(0), bias_diff); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shelhamer @slayton58 I am a bit confused here... Why do we need to zero out the diff? It confuses me as parameter gradients should be accumulated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is definitely a bug -- thanks for the fix in #3254.
This is the same as #2737 except for
cuDNNConvolutionLayer::Forward_gpu()
now that algo and workspace are determined inReshape()
.Thanks @slayton58 for the integration.