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

Fix conv2d with kernel 1x1 stride > 1 #1868

Merged
merged 1 commit into from Apr 15, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions tensorflow/core/kernels/conv_ops.cc
Expand Up @@ -272,6 +272,7 @@ struct LaunchConvOp<GPUDevice, T> {
if (use_cudnn) {
Tensor input = input_param;
if (filter.dim_size(0) == 1 && filter.dim_size(1) == 1 &&
row_stride == 1 && col_stride == 1 &&
data_format == FORMAT_NHWC) {
// 1x1 filter, so call cublas directly.
const uint64 m =
Expand Down
49 changes: 49 additions & 0 deletions tensorflow/core/kernels/eigen_spatial_convolutions_test.cc
Expand Up @@ -405,6 +405,55 @@ TEST(EigenSpatialConvolutionsTest, StridedSpatialConvolution) {
}
}


TEST(EigenSpatialConvolutionsTest, KernelSmallerThanStride) {
const int input_depth = 2;
const int input_rows = 3;
const int input_cols = 3;
const int num_batches = 5;
const int output_depth = 6;
const int patch_rows = 1;
const int patch_cols = 1;
const int output_rows = 2;
const int output_cols = 2;

Tensor<float, 4> input(input_depth, input_rows, input_cols, num_batches);
Tensor<float, 4> kernel(output_depth, input_depth, patch_rows, patch_cols);
Tensor<float, 4> result(output_depth, output_rows, output_cols, num_batches);
input = input.constant(11.0f) + input.random();
kernel = kernel.constant(2.0f) + kernel.random();
result.setRandom();

// Apply a spatial convolution using a 1x1 kernel, valid padding, and a stride
// of 2.
int stride = 2;
result = SpatialConvolution(input, kernel, stride, stride, PADDING_VALID);

EXPECT_EQ(result.dimension(0), output_depth);
EXPECT_EQ(result.dimension(1), output_rows);
EXPECT_EQ(result.dimension(2), output_cols);
EXPECT_EQ(result.dimension(3), num_batches);

for (int b = 0; b < num_batches; ++b) {
for (int od = 0; od < output_depth; ++od) {
for (int i = 0; i < output_rows; ++i) {
for (int j = 0; j < output_cols; ++j) {
float expected = 0.0f;
for (int c = 0; c < patch_cols; ++c) {
for (int r = 0; r < patch_rows; ++r) {
for (int id = 0; id < input_depth; ++id) {
expected += input(id, r + stride * i, c + stride * j, b) *
kernel(od, id, r, c);
}
}
}
EigenApprox(result(od, i, j, b), expected);
}
}
}
}
}

TEST(EigenSpatialConvolutionsTest, StridedSpatialConvolutionRowMajor) {
const int input_depth = 10;
const int input_rows = 5;
Expand Down
5 changes: 0 additions & 5 deletions tensorflow/core/kernels/ops_util.cc
Expand Up @@ -38,11 +38,6 @@ Status Get2dOutputSizeVerbose(const int in_height, const int in_width,
int row_stride, int col_stride, Padding padding,
int* new_height, int* new_width, int* pad_top,
int* pad_bottom, int* pad_left, int* pad_right) {
// Cannot have strides larger than the patch size.
if (row_stride > filter_height || col_stride > filter_width) {
return errors::InvalidArgument(
"stride must be less than or equal to kernel size");
}
switch (padding) {
case Padding::VALID:
*new_height = ceil((in_height - filter_height + 1.f) /
Expand Down
6 changes: 0 additions & 6 deletions tensorflow/core/kernels/ops_util_test.cc
Expand Up @@ -129,12 +129,6 @@ class OpsUtilTest : public ::testing::Test {
}
};

// Test stride > ksize fails with INVALID_ARGUMENT.
TEST_F(OpsUtilTest, Get2dOutputSizeInvalidTest) {
padding_struct pad_struct = {{3, 3, 1, 2, 2, 2, SAME}, {3, 3, 1, 1, 1, 1}};
VerifyGet2dOutputSizeBoundaries(pad_struct, error::INVALID_ARGUMENT);
}

TEST_F(OpsUtilTest, Get2dOutputSizeNegativeSizeTest) {
padding_struct pad_struct = {{1, 1, 3, 3, 1, 1, VALID}, {-1, -1, 0, 0, 0, 0}};
VerifyGet2dOutputSizeBoundaries(pad_struct, error::INVALID_ARGUMENT);
Expand Down
29 changes: 14 additions & 15 deletions tensorflow/python/kernel_tests/conv_ops_test.py
Expand Up @@ -319,6 +319,20 @@ def testConv2D2x2FilterStride1x2(self):
strides=[1, 2], padding="VALID",
expected=expected_output)

def testConv2DKernelSmallerThanStrideValid(self):
expected_output = [65, 95, 275, 305]
self._VerifyValues(tensor_in_sizes=[1, 7, 7, 1],
filter_in_sizes=[2, 2, 1, 1],
strides=[3, 3], padding="VALID",
expected=expected_output)

def testConv2DKernelSmallerThanStrideSame(self):
expected_output = [1, 3, 7, 9]
self._VerifyValues(tensor_in_sizes=[1, 3, 3, 1],
filter_in_sizes=[1, 1, 1, 1],
strides=[2, 2], padding="SAME",
expected=expected_output)

Copy link

Choose a reason for hiding this comment

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

Might also be nice to have a test with unequal strides > filter sizes, just to make sure the arguments are correctly propagated consistently. Optional.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh forgot to do that... but maybe i'll pass on that for now if you don't mind. I'm curious to see if the build bots pass the tests, as my GPU/CUDA setup is a bit wacky.

Copy link

Choose a reason for hiding this comment

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

That's fine, now that I think about it, this isn't really taking any new code paths and we should have coverage for the non-equal stride case already.

# Testing for backprops
def _RunAndVerifyBackpropInput(self, input_sizes, filter_sizes, output_sizes,
strides, padding, expected, data_format,
Expand Down Expand Up @@ -862,21 +876,6 @@ def testShapeFunctionEdgeCases(self):
shape=[21, 20, 3, 2]),
strides=[1, 1, 1, 1], padding="SAME")

# Stride larger than filter.
with self.assertRaisesRegexp(ValueError,
"stride must be less than or equal to filter"):
tf.nn.conv2d(tf.placeholder(tf.float32,
shape=[32, 20, 20, 3]),
tf.placeholder(tf.float32,
shape=[4, 5, 3, 2]),
strides=[1, 5, 5, 1], padding="SAME")
with self.assertRaisesRegexp(ValueError,
"stride must be less than or equal to filter"):
tf.nn.conv2d(tf.placeholder(tf.float32,
shape=[32, 20, 20, 3]),
tf.placeholder(tf.float32,
shape=[5, 4, 3, 2]),
strides=[1, 5, 5, 1], padding="SAME")


# This is only a very simple test. More comprehensive tests live in
Expand Down
41 changes: 27 additions & 14 deletions tensorflow/python/kernel_tests/pooling_ops_test.py
Expand Up @@ -370,6 +370,33 @@ def testDepthwiseMaxPool2x2DepthWindow3(self):
expected=[3.0, 6.0, 9.0, 12.0, 15.0, 18.0, 21.0, 24.0],
use_gpu=False)

def testKernelSmallerThanStride(self):
for use_gpu in [True, False]:
self._VerifyValues(tf.nn.max_pool, input_sizes=[1, 3, 3, 1],
ksize=[1, 1, 1, 1], strides=[1, 2, 2, 1],
padding="SAME",
expected=[1, 3, 7, 9],
use_gpu=use_gpu)

self._VerifyValues(tf.nn.max_pool, input_sizes=[1, 7, 7, 1],
ksize=[1, 2, 2, 1], strides=[1, 3, 3, 1],
padding="VALID",
expected=[9, 12, 30, 33],
use_gpu=use_gpu)

self._VerifyValues(tf.nn.avg_pool, input_sizes=[1, 3, 3, 1],
ksize=[1, 1, 1, 1], strides=[1, 2, 2, 1],
padding="SAME",
expected=[1, 3, 7, 9],
use_gpu=use_gpu)

self._VerifyValues(tf.nn.avg_pool, input_sizes=[1, 7, 7, 1],
ksize=[1, 2, 2, 1], strides=[1, 3, 3, 1],
padding="VALID",
expected=[5, 8, 26, 29],
use_gpu=use_gpu)


def _testDepthwiseMaxPoolInvalidConfig(self, in_size, ksize, strides,
error_msg, use_gpu=False):
t = tf.constant(1.0, shape=in_size)
Expand Down Expand Up @@ -885,20 +912,6 @@ def testShapeFunctionEdgeCases(self):
shape=[32, 20, 20, 3]),
ksize=[1, 21, 20, 1], strides=[1, 1, 1, 1], padding="SAME")

# Stride larger than filter.
for pool_func in [tf.nn.max_pool, tf.nn.avg_pool,
tf.nn.max_pool_with_argmax]:
with self.assertRaisesRegexp(
ValueError, "stride must be less than or equal to filter"):
pool_func(tf.placeholder(tf.float32,
shape=[32, 20, 20, 3]),
ksize=[1, 5, 3, 1], strides=[1, 5, 5, 1], padding="SAME")
with self.assertRaisesRegexp(
ValueError, "stride must be less than or equal to filter"):
pool_func(tf.placeholder(tf.float32,
shape=[32, 20, 20, 3]),
ksize=[1, 3, 5, 1], strides=[1, 5, 5, 1], padding="SAME")


def GetMaxPoolFwdTest(input_size, filter_size, strides, padding):
def Test(self):
Expand Down
4 changes: 0 additions & 4 deletions tensorflow/python/ops/common_shapes.py
Expand Up @@ -149,10 +149,6 @@ def get2d_conv_output_size(input_height, input_width, filter_height,
"filter must not be larger than the input: "
"Filter: [%sx%s] Input: [%sx%s]"
% (filter_height, filter_width, input_height, input_width))
if row_stride > filter_height or col_stride > filter_width:
raise ValueError("stride must be less than or equal to filter size",
"stride: [%sx%s] filter: [%sx%s]"
% (row_stride, col_stride, filter_height, filter_width))

# Compute number of rows in the output, based on the padding.
if input_height.value is None or filter_height.value is None:
Expand Down