Fix convolution for non-square kernels

HISTORY.md

@@ -1,5 +1,6 @@
 ### mlpack ?.?.?
 ###### ????-??-??
+  * Bugfix for non-square convolution kernels (#3376).
 
 ### mlpack 4.0.1
 ###### 2022-12-23

src/mlpack/methods/ann/layer/convolution_impl.hpp

@@ -384,8 +384,8 @@ void ConvolutionType<
       (padWLeft != 0 || padWRight != 0 || padHTop != 0 || padHBottom != 0);
 
   // To perform the backward pass, we need to rotate all the filters.
-  arma::Cube<typename MatType::elem_type> rotatedFilters(weight.n_cols,
-      weight.n_rows, weight.n_slices);
+  arma::Cube<typename MatType::elem_type> rotatedFilters(weight.n_rows,
+      weight.n_cols, weight.n_slices);
 
   // To perform the backward pass, we need to dilate all the mappedError.
   arma::Cube<typename MatType::elem_type> dilatedMappedError;

src/mlpack/methods/ann/layer/grouped_convolution_impl.hpp

@@ -402,8 +402,8 @@ void GroupedConvolutionType<
       (padWLeft != 0 || padWRight != 0 || padHTop != 0 || padHBottom != 0);
 
   // To perform the backward pass, we need to rotate all the filters.
-  arma::Cube<typename MatType::elem_type> rotatedFilters(weight.n_cols,
-      weight.n_rows, weight.n_slices);
+  arma::Cube<typename MatType::elem_type> rotatedFilters(weight.n_rows,
+      weight.n_cols, weight.n_slices);
 
   #pragma omp parallel for
   for (size_t map = 0; map < ((maps * inMaps) / groups); ++map)

src/mlpack/tests/ann/layer/convolution.cpp

@@ -441,3 +441,23 @@ TEST_CASE("AdvancedConvolutionLayerWithStrideTest", "[ANNLayerTest]")
   layer.Backward(input, output, delta);
   REQUIRE(arma::accu(delta) == Approx(115.3515701294).epsilon(1e-5));
 }
+
+// Make a simple convolutional layer with non-square filters, and make sure the
+// forward and backward and gradient passes all return a result.  (This checks
+// that we don't have any shape errors.)
+TEST_CASE("NonSquareConvolutionTest", "[ANNLayerTest]")
+{
+  Convolution module1(1, 5, 3);
+  module1.InputDimensions() = std::vector<size_t>({ 7, 7 });
+  module1.ComputeOutputDimensions();
+  arma::mat weights1(module1.WeightSize(), 1);
+  module1.SetWeights(weights1.memptr());
+
+  arma::mat data(49, 10, arma::fill::randu);
+  arma::mat forwardResult(module1.OutputSize(), 10, arma::fill::zeros);
+  REQUIRE_NOTHROW(module1.Forward(data, forwardResult));
+  arma::mat backwardResult(49, 10);
+  REQUIRE_NOTHROW(module1.Backward(data, forwardResult, backwardResult));
+  arma::mat gradientResult(module1.WeightSize(), 1);
+  REQUIRE_NOTHROW(module1.Gradient(data, backwardResult, gradientResult));
+}

src/mlpack/tests/ann/layer/grouped_convolution.cpp

@@ -202,3 +202,23 @@ TEST_CASE("GradientGroupedConvolutionLayerTest", "[ANNLayerTest]")
 
   REQUIRE(CheckGradient(function) < 1e-1);
 }
+
+// Make a simple grouped convolutional layer with non-square filters, and make
+// sure the forward and backward and gradient passes all return a result.  (This
+// checks that we don't have any shape errors.)
+TEST_CASE("NonSquareGroupedConvolutionTest", "[ANNLayerTest]")
+{
+  GroupedConvolution module1(1, 5, 3, 1);
+  module1.InputDimensions() = std::vector<size_t>({ 7, 7 });
+  module1.ComputeOutputDimensions();
+  arma::mat weights1(module1.WeightSize(), 1);
+  module1.SetWeights(weights1.memptr());
+
+  arma::mat data(49, 10, arma::fill::randu);
+  arma::mat forwardResult(module1.OutputSize(), 10, arma::fill::zeros);
+  REQUIRE_NOTHROW(module1.Forward(data, forwardResult));
+  arma::mat backwardResult(49, 10);
+  REQUIRE_NOTHROW(module1.Backward(data, forwardResult, backwardResult));
+  arma::mat gradientResult(module1.WeightSize(), 1);
+  REQUIRE_NOTHROW(module1.Gradient(data, backwardResult, gradientResult));
+}