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

Don't use cudnnConvolutionBiasActivationForward. #54

Merged
merged 3 commits into from
Oct 27, 2018

Conversation

galv
Copy link

@galv galv commented Oct 26, 2018

It supports only the precomputed implicit GEMM implementation of
convolution.

Also fix the bias layout.

It supports only the precomputed implicit GEMM implementation of
convolution.

// We pad the bias with leading dims of 1, since CUDNN's tensors appear to
// need a dimension of at least 3.
int bias_dims[3] = {1, 1, c.num_channels_out};
int bias_stride[3] = {c.num_channels_out, c.num_channels_out, 1};
int bias_dims[4] = {1, c.num_channels_out, 1, 1};
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain - preferably in a comment - why this specific pattern of dims is required? It's mysterious to me.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. I can make the bias optional at some point later.

The ConvolutionComputation class does not know whether or not the bias
is optional at initialization time. It will simply avoid using the bias
if it is a nullptr.
@galv
Copy link
Author

galv commented Oct 27, 2018

@danpovey I made the bias optional in my latest commit. The ConvolutionComputation object doesn't know whether the bias is optional in my design. It will simply avoid running any bias-related routines if the bias argument points to null. If you want the ConvolutionComputation object to take a boolean specifying whether or not the bias is used, and check that that matches with whether the bias is null or not, feel free to implement it that way.

@danpovey
Copy link
Owner

Thanks-- merging. Try to get in the habit of changing the documentation when you make code changes! It's easier psychologically to do it right away than later.

@danpovey danpovey merged commit 464db5c into danpovey:cudnn Oct 27, 2018
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.

2 participants