From 0b5af9f2751ad6cb7ce76c577a6e67abe6dc8aa1 Mon Sep 17 00:00:00 2001 From: Georgios Pinitas Date: Fri, 19 Jun 2020 23:22:08 +0100 Subject: 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 Reviewed-on: https://review.mlplatform.org/c/ml/ComputeLibrary/+/3427 Tested-by: Arm Jenkins Reviewed-by: Michele Di Giorgio Comments-Addressed: Arm Jenkins --- .../kernels/CLWidthConcatenate2TensorsKernel.cpp | 4 +- src/core/SubTensorInfo.cpp | 18 ++-- src/core/Validate.cpp | 12 +-- tests/framework/Asserts.h | 43 ++++++++- tests/validation/UNIT/SubTensorInfo.cpp | 104 +++++++++++++++++++++ 5 files changed, 164 insertions(+), 17 deletions(-) create mode 100644 tests/validation/UNIT/SubTensorInfo.cpp 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 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(parent_shape[i])) || (coords[i] + static_cast(shape[i]) > static_cast(parent_shape[i]))), - function, file, line); + const bool invalid_idx = coords[i] >= static_cast(parent_shape[i]); + const bool out_of_bounds_size = coords[i] + static_cast(shape[i]) > static_cast(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 -- cgit v1.2.1