COMPMID-3478: Allow SubTensors with XY indexing

Remove limitations on sub-tensor creation and allow any possible
indexing as long as it honors the parent tensor shape.

In case of padding expansion on a subtensor, an error is raised if
the sub-tensor is indexed on the XY dimensions.

Change-Id: Ibb5183a6cb7421f55068b47c06b43ebde0f6e9a5
Signed-off-by: Georgios Pinitas <georgios.pinitas@arm.com>
Reviewed-on: https://review.mlplatform.org/c/ml/ComputeLibrary/+/3427
Tested-by: Arm Jenkins <bsgcomp@arm.com>
Reviewed-by: Michele Di Giorgio <michele.digiorgio@arm.com>
Comments-Addressed: Arm Jenkins <bsgcomp@arm.com>

5 files changed, 164 insertions, 17 deletions

diff --git a/src/core/CL/kernels/CLWidthConcatenate2TensorsKernel.cpp b/src/core/CL/kernels/CLWidthConcatenate2TensorsKernel.cpp
index aba2af1bb7..ada84db285 100644
--- a/src/core/CL/kernels/CLWidthConcatenate2TensorsKernel.cpp
+++ b/src/core/CL/kernels/CLWidthConcatenate2TensorsKernel.cpp
@@ -51,8 +51,8 @@ std::pair<Status, Window> validate_and_configure_window(ITensorInfo *input1, ITe
     // The window needs to be based on the output
     Window             win = calculate_max_window(*output, Steps(num_elems_processed_per_iteration));
     AccessWindowStatic input1_access(input1, 0, 0, ceil_to_multiple(input1->dimension(0), num_elems_processed_per_iteration), input1->dimension(1));
-    const unsigned int input2_right_padding = (output->dimension(0) / num_elems_processed_per_iteration) * num_elems_processed_per_iteration - input1->dimension(
-                                                  0) + num_elems_processed_per_iteration - input2->dimension(0);
+    const unsigned int input2_right_padding = ((output->dimension(0) / num_elems_processed_per_iteration) * num_elems_processed_per_iteration - input1->dimension(0) - input2->dimension(
+                                                   0)) % num_elems_processed_per_iteration;
     AccessWindowStatic input2_access(input2, -(input1->dimension(0) % num_elems_processed_per_iteration),
                                      0, input2->dimension(0) + input2_right_padding, input2->dimension(1));
     AccessWindowHorizontal output_access(output, 0, num_elems_processed_per_iteration);
diff --git a/src/core/SubTensorInfo.cpp b/src/core/SubTensorInfo.cpp
index be8560fd61..a50f584023 100644
--- a/src/core/SubTensorInfo.cpp
+++ b/src/core/SubTensorInfo.cpp
@@ -41,12 +41,6 @@ namespace
  */
 TensorShape extend_parent_shape(TensorShape parent_shape, TensorShape shape, Coordinates coords)
 {
-    // Subtensor should not index in x, y dimensions.
-    ARM_COMPUTE_ERROR_ON((coords.x() != 0) || (coords.y() != 0));
-
-    // Cannot extend on x, y ?
-    ARM_COMPUTE_ERROR_ON((parent_shape.total_size() != 0) && (parent_shape.x() != shape.x()) && (parent_shape.y() != shape.y()));
-
     // Extend shape
     for(unsigned int i = 0; i < TensorShape::num_max_dimensions; ++i)
     {
@@ -70,6 +64,7 @@ SubTensorInfo::SubTensorInfo(ITensorInfo *parent, TensorShape tensor_shape, Coor
     : _parent(parent), _tensor_shape(tensor_shape), _coords(coords), _valid_region{ Coordinates(), _tensor_shape }, _extend_parent(extend_parent)
 {
     ARM_COMPUTE_ERROR_ON(parent == nullptr);
+
     // Check if subtensor is valid if parent is configured
     if(parent->tensor_shape().total_size() != 0 && !_extend_parent)
     {
@@ -118,6 +113,17 @@ bool SubTensorInfo::extend_padding(const PaddingSize &padding)
     ARM_COMPUTE_ERROR_ON(!_parent->is_resizable());
     ARM_COMPUTE_ERROR_ON(_parent->total_size() == 0);
 
+    // Check that you do not extend padding on sub-tensors unless XY shape matches parent tensor
+    // TODO(COMPMID-3558): Remove _extend_parent check
+    if(!_extend_parent && (padding.left || padding.right))
+    {
+        ARM_COMPUTE_ERROR_ON(_parent->tensor_shape().x() != tensor_shape().x());
+    }
+    if(!_extend_parent && (padding.top || padding.bottom))
+    {
+        ARM_COMPUTE_ERROR_ON(_parent->tensor_shape().y() != tensor_shape().y());
+    }
+
     // Extend parent padding if required
     return _parent->extend_padding(padding);
 }
diff --git a/src/core/Validate.cpp b/src/core/Validate.cpp
index f9bd6d6a45..cc80611107 100644
--- a/src/core/Validate.cpp
+++ b/src/core/Validate.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016-2019 ARM Limited.
+ * Copyright (c) 2016-2020 ARM Limited.
  *
  * SPDX-License-Identifier: MIT
  *
@@ -176,16 +176,12 @@ arm_compute::Status arm_compute::error_on_unconfigured_kernel(const char *functi
 arm_compute::Status arm_compute::error_on_invalid_subtensor(const char *function, const char *file, const int line,
                                                             const TensorShape &parent_shape, const Coordinates &coords, const TensorShape &shape)
 {
-    // Subtensor should not index in x, y dimensions.
-    ARM_COMPUTE_RETURN_ERROR_ON_LOC(((coords.x() != 0) || (coords.y() != 0)), function, file, line);
-    // Subtensor shape should match parent tensor in x, y dimensions.
-    ARM_COMPUTE_RETURN_ERROR_ON_LOC(((parent_shape.x() != shape.x()) || (parent_shape.y() != shape.y())), function, file, line);
-
     // Check dimensions
     for(unsigned int i = 0; i < TensorShape::num_max_dimensions; ++i)
     {
-        ARM_COMPUTE_RETURN_ERROR_ON_LOC(((coords[i] >= static_cast<int>(parent_shape[i])) || (coords[i] + static_cast<int>(shape[i]) > static_cast<int>(parent_shape[i]))),
-                                        function, file, line);
+        const bool invalid_idx        = coords[i] >= static_cast<int>(parent_shape[i]);
+        const bool out_of_bounds_size = coords[i] + static_cast<int>(shape[i]) > static_cast<int>(parent_shape[i]);
+        ARM_COMPUTE_RETURN_ERROR_ON_LOC(invalid_idx || out_of_bounds_size, function, file, line);
     }
     return arm_compute::Status{};
 }
diff --git a/tests/framework/Asserts.h b/tests/framework/Asserts.h
index 9d6d4fad9a..3b91b324b3 100644
--- a/tests/framework/Asserts.h
+++ b/tests/framework/Asserts.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017 ARM Limited.
+ * Copyright (c) 2017-2020 ARM Limited.
  *
  * SPDX-License-Identifier: MIT
  *
@@ -135,6 +135,47 @@ ARM_COMPUTE_TEST_COMP_FACTORY(ASSERT, Assertion, !=, NOT_EQUAL, throw arm_comput
         arm_compute::test::framework::Framework::get().clear_test_info();                                                                     \
     } while(false)
 
+#define ARM_COMPUTE_EXPECT_NO_THROW(X, LEVEL)                                                                                                 \
+    do                                                                                                                                        \
+    {                                                                                                                                         \
+        try                                                                                                                                   \
+        {                                                                                                                                     \
+            const auto &x = X;                                                                                                                \
+            (void)x;                                                                                                                          \
+        }                                                                                                                                     \
+        catch(...)                                                                                                                            \
+        {                                                                                                                                     \
+            std::stringstream msg;                                                                                                            \
+            msg << "Expectation '" #X "' to not throw failed.\n";                                                                             \
+            arm_compute::test::framework::Framework::get().print_test_info(msg);                                                              \
+            arm_compute::test::framework::Framework::get().log_failed_expectation(arm_compute::test::framework::TestError(msg.str(), LEVEL)); \
+        }                                                                                                                                     \
+        arm_compute::test::framework::Framework::get().clear_test_info();                                                                     \
+    } while(false)
+
+#define ARM_COMPUTE_EXPECT_THROW(X, LEVEL)                                                                                                    \
+    do                                                                                                                                        \
+    {                                                                                                                                         \
+        bool exception_caught = false;                                                                                                        \
+        try                                                                                                                                   \
+        {                                                                                                                                     \
+            const auto &x = X;                                                                                                                \
+            (void)x;                                                                                                                          \
+        }                                                                                                                                     \
+        catch(...)                                                                                                                            \
+        {                                                                                                                                     \
+            exception_caught = true;                                                                                                          \
+        }                                                                                                                                     \
+        if(!exception_caught)                                                                                                                 \
+        {                                                                                                                                     \
+            std::stringstream msg;                                                                                                            \
+            msg << "Expectation '" #X "' to throw failed.\n";                                                                                 \
+            arm_compute::test::framework::Framework::get().print_test_info(msg);                                                              \
+            arm_compute::test::framework::Framework::get().log_failed_expectation(arm_compute::test::framework::TestError(msg.str(), LEVEL)); \
+        }                                                                                                                                     \
+        arm_compute::test::framework::Framework::get().clear_test_info();                                                                     \
+    } while(false)
+
 #define ARM_COMPUTE_ASSERT_FAIL(MSG)                                                                              \
     do                                                                                                            \
     {                                                                                                             \
diff --git a/tests/validation/UNIT/SubTensorInfo.cpp b/tests/validation/UNIT/SubTensorInfo.cpp
new file mode 100644
index 0000000000..7f5cdd78e5
--- /dev/null
+++ b/tests/validation/UNIT/SubTensorInfo.cpp
@@ -0,0 +1,104 @@
+/*
+ * Copyright (c) 2020 ARM Limited.
+ *
+ * SPDX-License-Identifier: MIT
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
+ * deal in the Software without restriction, including without limitation the
+ * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in all
+ * copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+#include "arm_compute/core/SubTensorInfo.h"
+#include "arm_compute/core/TensorInfo.h"
+#include "arm_compute/core/Types.h"
+#include "tests/framework/Asserts.h"
+#include "tests/framework/Macros.h"
+#include "tests/validation/Validation.h"
+#include "utils/TypePrinter.h"
+
+namespace arm_compute
+{
+namespace test
+{
+namespace validation
+{
+TEST_SUITE(UNIT)
+TEST_SUITE(SubTensorInfo)
+
+/** Validate sub-tensor creation
+ *
+ * Test performed:
+ *
+ *  - Negative testing on X indexing
+ *  - Negative testing on Y indexing
+ *  - Positive testing by indexing on X,Y indexing
+ * */
+TEST_CASE(SubTensorCreation, framework::DatasetMode::ALL)
+{
+    // Create tensor info
+    TensorInfo info(TensorShape(23U, 17U, 3U), 1, DataType::F32);
+
+    // Negative testing on X
+    ARM_COMPUTE_EXPECT_THROW(SubTensorInfo(&info, TensorShape(13U, 17U, 3U), Coordinates(24, 0, 0)), framework::LogLevel::ERRORS);
+    ARM_COMPUTE_EXPECT_THROW(SubTensorInfo(&info, TensorShape(13U, 17U, 3U), Coordinates(15, 0, 0)), framework::LogLevel::ERRORS);
+
+    // Negative testing on Y
+    ARM_COMPUTE_EXPECT_THROW(SubTensorInfo(&info, TensorShape(23U, 8U, 3U), Coordinates(0, 18, 0)), framework::LogLevel::ERRORS);
+    ARM_COMPUTE_EXPECT_THROW(SubTensorInfo(&info, TensorShape(23U, 8U, 3U), Coordinates(0, 13, 0)), framework::LogLevel::ERRORS);
+
+    // Positive testing on XY indexing
+    ARM_COMPUTE_EXPECT_NO_THROW(SubTensorInfo(&info, TensorShape(4U, 3U, 2U), Coordinates(5, 2, 1)), framework::LogLevel::ERRORS);
+}
+
+/** Validate when extending padding on sub-tensor
+ *
+ * Tests performed:
+ *  - A) Extend padding when SubTensor XY does not match parent tensor should fail
+ *    B) Extend with zero padding when SubTensor XY does not match parent tensor should succeed
+ *  - C) Extend padding when SubTensor XY matches parent tensor should succeed
+ */
+TEST_CASE(SubTensorPaddingExpansion, framework::DatasetMode::ALL)
+{
+    // Test A
+    {
+        TensorInfo    tensor_info(TensorShape(23U, 17U, 3U), 1, DataType::F32);
+        SubTensorInfo sub_tensor_info(&tensor_info, TensorShape(4U, 3U, 2U), Coordinates(5, 2, 1));
+        ARM_COMPUTE_EXPECT_THROW(sub_tensor_info.extend_padding(PaddingSize(2, 1)), framework::LogLevel::ERRORS);
+    }
+
+    // Test B
+    {
+        TensorInfo    tensor_info(TensorShape(23U, 17U, 3U), 1, DataType::F32);
+        SubTensorInfo sub_tensor_info(&tensor_info, TensorShape(4U, 3U, 1U), Coordinates(5, 2, 1));
+        ARM_COMPUTE_EXPECT_NO_THROW(sub_tensor_info.extend_padding(PaddingSize(0, 0)), framework::LogLevel::ERRORS);
+        ARM_COMPUTE_EXPECT(tensor_info.padding().uniform(), framework::LogLevel::ERRORS);
+    }
+
+    // Test C
+    {
+        TensorInfo    tensor_info(TensorShape(23U, 17U, 3U), 1, DataType::F32);
+        SubTensorInfo sub_tensor_info(&tensor_info, TensorShape(23U, 17U, 1U), Coordinates(0, 0, 1));
+        ARM_COMPUTE_EXPECT_NO_THROW(sub_tensor_info.extend_padding(PaddingSize(2, 1)), framework::LogLevel::ERRORS);
+        ARM_COMPUTE_EXPECT(tensor_info.padding().top == 2, framework::LogLevel::ERRORS);
+        ARM_COMPUTE_EXPECT(tensor_info.padding().right == 1, framework::LogLevel::ERRORS);
+    }
+}
+
+TEST_SUITE_END() // SubTensorInfo
+TEST_SUITE_END()
+} // namespace validation
+} // namespace test
+} // namespace arm_compute