From bdaa44d0be7db35e08b0367515def2cb1dfab1a9 Mon Sep 17 00:00:00 2001 From: Egor Shulman Date: Wed, 15 Sep 2021 07:52:46 +0300 Subject: [PATCH] Fixed Minimum op if u8/16/32/64 data type is used (#6665) --- .../tests/functional/op_reference/minimum.cpp | 112 ++++++++++++++++++ .../src/mkldnn_plugin/mkldnn_plugin.cpp | 2 + .../convert_minimum_to_power_and_max.cpp | 2 +- ngraph/core/src/op/minimum.cpp | 2 + ngraph/test/CMakeLists.txt | 1 - ngraph/test/backend/minimum.in.cpp | 99 ---------------- ngraph/test/runtime/ie/unit_test.manifest | 2 - ngraph/test/util/engine/ie_engines.cpp | 99 ++++++++++++++-- runtime/bindings/python/tests/__init__.py | 1 - .../python/tests/test_onnx/test_backend.py | 8 -- 10 files changed, 205 insertions(+), 123 deletions(-) create mode 100644 docs/template_plugin/tests/functional/op_reference/minimum.cpp delete mode 100644 ngraph/test/backend/minimum.in.cpp diff --git a/docs/template_plugin/tests/functional/op_reference/minimum.cpp b/docs/template_plugin/tests/functional/op_reference/minimum.cpp new file mode 100644 index 00000000000..4dfeb44a532 --- /dev/null +++ b/docs/template_plugin/tests/functional/op_reference/minimum.cpp @@ -0,0 +1,112 @@ +// Copyright (C) 2018-2021 Intel Corporation +// SPDX-License-Identifier: Apache-2.0 +// + +#include + +#include +#include +#include +#include +#include + +#include "base_reference_test.hpp" + +using namespace ngraph; +using namespace InferenceEngine; +using namespace reference_tests; + +struct MinimumParams { + template + MinimumParams(const PartialShape& s, + const element::Type& iType, const element::Type& oType, + const std::vector& iValues1, const std::vector& iValues2, + const std::vector& oValues) + : pshape(s), + inType(iType), + outType(oType), + inputData1(CreateBlob(iType, iValues1)), + inputData2(CreateBlob(iType, iValues2)), + refData(CreateBlob(oType, oValues)) {} + PartialShape pshape; + element::Type inType; + element::Type outType; + Blob::Ptr inputData1; + Blob::Ptr inputData2; + Blob::Ptr refData; +}; + +class ReferenceMinimumLayerTest : public testing::TestWithParam, public CommonReferenceTest { +public: + void SetUp() override { + auto params = GetParam(); + function = CreateFunction(params.pshape, params.inType); + inputData = {params.inputData1, params.inputData2}; + refOutData = {params.refData}; + } + static std::string getTestCaseName(const testing::TestParamInfo& obj) { + auto param = obj.param; + std::ostringstream result; + result << "shape=" << param.pshape << "_"; + result << "iType=" << param.inType << "_"; + result << "oType=" << param.outType; + return result.str(); + } + +private: + static std::shared_ptr CreateFunction(const PartialShape& shape, const element::Type& data_type) { + auto A = std::make_shared(data_type, shape); + auto B = std::make_shared(data_type, shape); + return std::make_shared(std::make_shared(A, B), ParameterVector{A, B}); + } +}; + +TEST_P(ReferenceMinimumLayerTest, CompareWithHardcodedRefs) { + Exec(); +} + +INSTANTIATE_TEST_SUITE_P( + smoke_Minimum, ReferenceMinimumLayerTest, ::testing::Values( + MinimumParams(PartialShape {8}, + element::u8, + element::u8, + std::vector {1, 8, 8, 17, 5, 5, 2, 3}, + std::vector {1, 2, 4, 8, 0, 2, 1, 200}, + std::vector {1, 2, 4, 8, 0, 2, 1, 3}), + MinimumParams(PartialShape {8}, + element::u16, + element::u16, + std::vector {1, 8, 8, 17, 5, 7, 123, 3}, + std::vector {1, 2, 4, 8, 0, 2, 1, 1037}, + std::vector {1, 2, 4, 8, 0, 2, 1, 3}), + MinimumParams(PartialShape {8}, + element::u32, + element::u32, + std::vector {1, 8, 8, 17, 5, 5, 2, 1}, + std::vector {1, 2, 4, 8, 0, 2, 1, 222}, + std::vector {1, 2, 4, 8, 0, 2, 1, 1}), + MinimumParams(PartialShape {8}, + element::u64, + element::u64, + std::vector {1, 8, 8, 17, 5, 5, 2, 13}, + std::vector {1, 2, 4, 8, 0, 2, 1, 2222}, + std::vector {1, 2, 4, 8, 0, 2, 1, 13}), + MinimumParams(PartialShape {8}, + element::f32, + element::f32, + std::vector {1, 8, -8, 17, -0.5, 0.5, 2, 1}, + std::vector {1, 2, 4, 8, 0, 0, 1, 1.5}, + std::vector {1, 2, -8, 8, -.5, 0, 1, 1}), + MinimumParams(PartialShape {8}, + element::i32, + element::i32, + std::vector {1, 8, -8, 17, -5, 67635216, 2, 1}, + std::vector {1, 2, 4, 8, 0, 18448, 1, 6}, + std::vector {1, 2, -8, 8, -5, 18448, 1, 1}), + MinimumParams(PartialShape {8}, + element::i64, + element::i64, + std::vector {1, 8, -8, 17, -5, 67635216, 2, 17179887632}, + std::vector {1, 2, 4, 8, 0, 18448, 1, 280592}, + std::vector {1, 2, -8, 8, -5, 18448, 1, 280592})), + ReferenceMinimumLayerTest::getTestCaseName); diff --git a/inference-engine/src/mkldnn_plugin/mkldnn_plugin.cpp b/inference-engine/src/mkldnn_plugin/mkldnn_plugin.cpp index 65c25c6d060..5bb831e3407 100644 --- a/inference-engine/src/mkldnn_plugin/mkldnn_plugin.cpp +++ b/inference-engine/src/mkldnn_plugin/mkldnn_plugin.cpp @@ -62,6 +62,7 @@ #include #include #include +#include #include #include #include @@ -321,6 +322,7 @@ static void TransformationUpToCPUSpecificOpSet(std::shared_ptr pass_config->disable(); pass_config->disable(); pass_config->disable(); + pass_config->disable(); pass_config->enable(); pass_config->enable(); diff --git a/inference-engine/src/transformations/src/transformations/op_conversions/convert_minimum_to_power_and_max.cpp b/inference-engine/src/transformations/src/transformations/op_conversions/convert_minimum_to_power_and_max.cpp index 3ab36795350..02fa3590de2 100644 --- a/inference-engine/src/transformations/src/transformations/op_conversions/convert_minimum_to_power_and_max.cpp +++ b/inference-engine/src/transformations/src/transformations/op_conversions/convert_minimum_to_power_and_max.cpp @@ -20,7 +20,7 @@ ngraph::pass::ConvertMinimum::ConvertMinimum() { ngraph::matcher_pass_callback callback = [this](pattern::Matcher& m) { auto minimum = std::dynamic_pointer_cast (m.get_match_root()); - if (!minimum || transformation_callback(minimum)) { + if (!minimum || transformation_callback(minimum) || !minimum->get_output_element_type(0).is_signed()) { return false; } diff --git a/ngraph/core/src/op/minimum.cpp b/ngraph/core/src/op/minimum.cpp index 4a6f6d77c2c..ec195c5f775 100644 --- a/ngraph/core/src/op/minimum.cpp +++ b/ngraph/core/src/op/minimum.cpp @@ -41,6 +41,8 @@ bool evaluate_minimum(const HostTensorPtr& arg0, switch (arg0->get_element_type()) { NGRAPH_TYPE_CASE(evaluate_minimum, i32, arg0, arg1, out, broadcast_spec); NGRAPH_TYPE_CASE(evaluate_minimum, i64, arg0, arg1, out, broadcast_spec); + NGRAPH_TYPE_CASE(evaluate_minimum, u8, arg0, arg1, out, broadcast_spec); + NGRAPH_TYPE_CASE(evaluate_minimum, u16, arg0, arg1, out, broadcast_spec); NGRAPH_TYPE_CASE(evaluate_minimum, u32, arg0, arg1, out, broadcast_spec); NGRAPH_TYPE_CASE(evaluate_minimum, u64, arg0, arg1, out, broadcast_spec); NGRAPH_TYPE_CASE(evaluate_minimum, f16, arg0, arg1, out, broadcast_spec); diff --git a/ngraph/test/CMakeLists.txt b/ngraph/test/CMakeLists.txt index ca2d3c32e28..241c545e872 100644 --- a/ngraph/test/CMakeLists.txt +++ b/ngraph/test/CMakeLists.txt @@ -468,7 +468,6 @@ set(MULTI_TEST_SRC backend/matrix_nms.in.cpp backend/maximum.in.cpp backend/max_pool.in.cpp - backend/minimum.in.cpp backend/mish.in.cpp backend/mod.in.cpp backend/multiclass_nms.in.cpp diff --git a/ngraph/test/backend/minimum.in.cpp b/ngraph/test/backend/minimum.in.cpp deleted file mode 100644 index 3f5ec0a132d..00000000000 --- a/ngraph/test/backend/minimum.in.cpp +++ /dev/null @@ -1,99 +0,0 @@ -// Copyright (C) 2018-2021 Intel Corporation -// SPDX-License-Identifier: Apache-2.0 -// - -#include -#include -#include -#include -#include -#include - -// clang-format off -#ifdef ${BACKEND_NAME}_FLOAT_TOLERANCE_BITS -#define DEFAULT_FLOAT_TOLERANCE_BITS ${BACKEND_NAME}_FLOAT_TOLERANCE_BITS -#endif - -#ifdef ${BACKEND_NAME}_DOUBLE_TOLERANCE_BITS -#define DEFAULT_DOUBLE_TOLERANCE_BITS ${BACKEND_NAME}_DOUBLE_TOLERANCE_BITS -#endif -// clang-format on - -#include "gtest/gtest.h" -#include "ngraph/ngraph.hpp" -#include "util/engine/test_engines.hpp" -#include "util/test_case.hpp" -#include "util/test_control.hpp" - -using namespace std; -using namespace ngraph; - -static string s_manifest = "${MANIFEST}"; -using TestEngine = test::ENGINE_CLASS_NAME(${BACKEND_NAME}); - -NGRAPH_TEST(${BACKEND_NAME}, minimum) { - Shape shape{2, 2, 2}; - auto A = make_shared(element::f32, shape); - auto B = make_shared(element::f32, shape); - auto f = make_shared(make_shared(A, B), ParameterVector{A, B}); - - std::vector a{1, 8, -8, 17, -0.5, 0.5, 2, 1}; - std::vector b{1, 2, 4, 8, 0, 0, 1, 1.5}; - - auto test_case = test::TestCase(f); - test_case.add_multiple_inputs({a, b}); - test_case.add_expected_output(shape, {1, 2, -8, 8, -.5, 0, 1, 1}); - test_case.run(); -} - -NGRAPH_TEST(${BACKEND_NAME}, minimum_int32) { - Shape shape{2, 2, 2}; - auto A = make_shared(element::i32, shape); - auto B = make_shared(element::i32, shape); - auto f = make_shared(make_shared(A, B), ParameterVector{A, B}); - - std::vector a{1, 8, -8, 17, -5, 67635216, 2, 1}; - std::vector b{1, 2, 4, 8, 0, 18448, 1, 6}; - - auto test_case = test::TestCase(f); - test_case.add_multiple_inputs({a, b}); - test_case.add_expected_output(shape, {1, 2, -8, 8, -5, 18448, 1, 1}); - test_case.run(); -} - -NGRAPH_TEST(${BACKEND_NAME}, minimum_int64) { - Shape shape{2, 2, 2}; - auto A = make_shared(element::i64, shape); - auto B = make_shared(element::i64, shape); - auto f = make_shared(make_shared(A, B), ParameterVector{A, B}); - - std::vector a{1, 8, -8, 17, -5, 67635216, 2, 17179887632}; - std::vector b{1, 2, 4, 8, 0, 18448, 1, 280592}; - - auto test_case = test::TestCase(f); - test_case.add_multiple_inputs({a, b}); - test_case.add_expected_output(shape, {1, 2, -8, 8, -5, 18448, 1, 280592}); - test_case.run(); -} - -// TODO Refactor to use TestCase if u16 will be handled correctly -NGRAPH_TEST(${BACKEND_NAME}, minimum_u16) { - const Shape shape{3}; - const auto A = make_shared(element::u16, shape); - const auto B = make_shared(element::u16, shape); - auto f = make_shared(make_shared(A, B), ParameterVector{A, B}); - - auto backend = runtime::Backend::create("${BACKEND_NAME}"); - - // Create some tensors for input/output - auto a = backend->create_tensor(element::u16, shape); - copy_data(a, std::vector{3, 2, 1}); - auto b = backend->create_tensor(element::u16, shape); - copy_data(b, std::vector{1, 4, 4}); - auto result = backend->create_tensor(element::u16, shape); - - auto handle = backend->compile(f); - handle->call_with_validate({result}, {a, b}); - - EXPECT_TRUE(test::all_close((std::vector{1, 2, 1}), read_vector(result))); -} diff --git a/ngraph/test/runtime/ie/unit_test.manifest b/ngraph/test/runtime/ie/unit_test.manifest index 8ac6a7d0564..cb80e1bb26b 100644 --- a/ngraph/test/runtime/ie/unit_test.manifest +++ b/ngraph/test/runtime/ie/unit_test.manifest @@ -1493,8 +1493,6 @@ IE_GPU.onnx_model_gather_elements_float_3D_axis_2 # not supported fp16 blob input IE_CPU.evaluate_ctc_greedy_decoder_seq_len_f16 -# incorrect result for Minimum if u16 type is used -minimum_u16 IE_CPU/ElemTypesTests/1.onnx_test_add_abc_set_precission # not yet implemented on CPU/GPU Gather 7 diff --git a/ngraph/test/util/engine/ie_engines.cpp b/ngraph/test/util/engine/ie_engines.cpp index a1da94ee61d..4856492c17d 100644 --- a/ngraph/test/util/engine/ie_engines.cpp +++ b/ngraph/test/util/engine/ie_engines.cpp @@ -24,14 +24,97 @@ namespace const auto expected_data = expected->rmap(); const auto* computed_data_buffer = computed_data.template as(); - const auto* expected_data_buffer = expected_data.template as(); - std::vector computed_values(computed_data_buffer, computed_data_buffer + computed->size()); - std::vector expected_values(expected_data_buffer, - expected_data_buffer + computed->size()); - return std::make_pair(std::move(computed_values), std::move(expected_values)); + switch (static_cast(expected->getTensorDesc().getPrecision())) + { + case InferenceEngine::Precision::FP32: { + const auto* expected_data_buffer = expected_data.template as(); + std::vector expected_values(expected_data_buffer, + expected_data_buffer + computed->size()); + return std::make_pair(std::move(computed_values), std::move(expected_values)); + break; + } + case InferenceEngine::Precision::FP64: { + const auto *expected_data_buffer = expected_data.template as(); + std::vector expected_values(expected_data_buffer, + expected_data_buffer + computed->size()); + return std::make_pair(std::move(computed_values), std::move(expected_values)); + break; + } + case InferenceEngine::Precision::I8: { + const auto *expected_data_buffer = expected_data.template as(); + std::vector expected_values(expected_data_buffer, + expected_data_buffer + computed->size()); + return std::make_pair(std::move(computed_values), std::move(expected_values)); + break; + } + case InferenceEngine::Precision::I16: { + const auto *expected_data_buffer = expected_data.template as(); + std::vector expected_values(expected_data_buffer, + expected_data_buffer + computed->size()); + return std::make_pair(std::move(computed_values), std::move(expected_values)); + break; + } + case InferenceEngine::Precision::I32: { + const auto *expected_data_buffer = expected_data.template as(); + std::vector expected_values(expected_data_buffer, + expected_data_buffer + computed->size()); + return std::make_pair(std::move(computed_values), std::move(expected_values)); + break; + } + case InferenceEngine::Precision::I64: { + const auto *expected_data_buffer = expected_data.template as(); + std::vector expected_values(expected_data_buffer, + expected_data_buffer + computed->size()); + return std::make_pair(std::move(computed_values), std::move(expected_values)); + break; + } + case InferenceEngine::Precision::U8: { + const auto *expected_data_buffer = expected_data.template as(); + std::vector expected_values(expected_data_buffer, + expected_data_buffer + computed->size()); + return std::make_pair(std::move(computed_values), std::move(expected_values)); + break; + } + case InferenceEngine::Precision::U16: { + const auto *expected_data_buffer = expected_data.template as(); + std::vector expected_values(expected_data_buffer, + expected_data_buffer + computed->size()); + return std::make_pair(std::move(computed_values), std::move(expected_values)); + break; + } + case InferenceEngine::Precision::U32: { + const auto *expected_data_buffer = expected_data.template as(); + std::vector expected_values(expected_data_buffer, + expected_data_buffer + computed->size()); + return std::make_pair(std::move(computed_values), std::move(expected_values)); + break; + } + case InferenceEngine::Precision::U64: { + const auto *expected_data_buffer = expected_data.template as(); + std::vector expected_values(expected_data_buffer, + expected_data_buffer + computed->size()); + return std::make_pair(std::move(computed_values), std::move(expected_values)); + break; + } + case InferenceEngine::Precision::BOOL: { + const auto *expected_data_buffer = expected_data.template as(); + std::vector expected_values(expected_data_buffer, + expected_data_buffer + computed->size()); + return std::make_pair(std::move(computed_values), std::move(expected_values)); + break; + } + case InferenceEngine::Precision::BF16: { + const auto *expected_data_buffer = expected_data.template as(); + std::vector expected_values(expected_data_buffer, + expected_data_buffer + computed->size()); + return std::make_pair(std::move(computed_values), std::move(expected_values)); + break; + } + default: THROW_IE_EXCEPTION << "Not implemented yet"; + } } /// Compares two blobs containing floating point elements. @@ -87,12 +170,6 @@ namespace const size_t tolerance_bits) { const auto& computed_precision = computed->getTensorDesc().getPrecision(); - const auto& expected_precision = expected->getTensorDesc().getPrecision(); - - if (computed_precision != expected_precision) - { - return testing::AssertionFailure() << "Blob precisions mismatch"; - } switch (static_cast(computed_precision)) { diff --git a/runtime/bindings/python/tests/__init__.py b/runtime/bindings/python/tests/__init__.py index 4f07f874900..38ff1060da0 100644 --- a/runtime/bindings/python/tests/__init__.py +++ b/runtime/bindings/python/tests/__init__.py @@ -96,7 +96,6 @@ xfail_issue_44957 = xfail_test(reason="Expected: Unsupported dynamic op: NonZero xfail_issue_44958 = xfail_test(reason="Expected: Unsupported dynamic op: Interpolate") xfail_issue_44965 = xfail_test(reason="Expected: RuntimeError: value info has no element") xfail_issue_44968 = xfail_test(reason="Expected: Unsupported dynamic op: Squeeze") -xfail_issue_46762 = xfail_test(reason="Incorrect result of Minimum op if uint data type is used") xfail_issue_47323 = xfail_test(reason="RuntimeError: The plugin does not support FP64") xfail_issue_47337 = xfail_test(reason="RuntimeError: Unsupported dynamic ops: v1::OneHot") xfail_issue_33593 = xfail_test(reason="Current implementation of MaxPool doesn't support indices output") diff --git a/runtime/bindings/python/tests/test_onnx/test_backend.py b/runtime/bindings/python/tests/test_onnx/test_backend.py index fb5ca82b46e..ff93956d7c8 100644 --- a/runtime/bindings/python/tests/test_onnx/test_backend.py +++ b/runtime/bindings/python/tests/test_onnx/test_backend.py @@ -45,7 +45,6 @@ from tests import ( xfail_issue_44968, xfail_issue_45180, xfail_issue_45344, - xfail_issue_46762, xfail_issue_47323, xfail_issue_47337, xfail_issue_48052, @@ -183,13 +182,6 @@ tests_expected_to_fail = [ "OnnxBackendNodeModelTest.test_top_k_smallest_cpu", ), (xfail_issue_33633, "OnnxBackendNodeModelTest.test_maxpool_2d_dilations_cpu"), - ( - xfail_issue_46762, - "OnnxBackendNodeModelTest.test_min_uint8_cpu", - "OnnxBackendNodeModelTest.test_min_uint16_cpu", - "OnnxBackendNodeModelTest.test_min_uint32_cpu", - "OnnxBackendNodeModelTest.test_min_uint64_cpu", - ), ( xfail_issue_55760, "OnnxBackendNodeModelTest.test_argmax_negative_axis_keepdims_example_select_last_index_cpu",