nGraph: Fix TopK output sorting by index (#3092)

docs/ops/sort/TopK_1.md

@@ -10,7 +10,7 @@
 
 * *axis*
 
-  * **Description**: Specifies the axis along which 
+  * **Description**: Specifies the axis along which the values are retrieved.
   * **Range of values**: An integer. Negative value means counting dimension from the end.
   * **Type**: `int`
   * **Default value**: None
@@ -50,7 +50,15 @@ Output tensor is populated by values computes in the following way:
 
     output[i1, ..., i(axis-1), j, i(axis+1) ..., iN] = top_k(input[i1, ...., i(axis-1), :, i(axis+1), ..., iN]), k, sort, mode)
 
-So for each slice `input[i1, ...., i(axis-1), :, i(axis+1), ..., iN]` which represents 1D array, top_k value is computed individually. Sorting and minimum/maximum are controlled by `sort` and `mode` attributes.
+So for each slice `input[i1, ...., i(axis-1), :, i(axis+1), ..., iN]` which represents 1D array, top_k value is computed individually.
+
+Sorting and minimum/maximum are controlled by `sort` and `mode` attributes:
+  * *mode*=`max`, *sort*=`value` - descending by value
+  * *mode*=`max`, *sort*=`index` - ascending by index
+  * *mode*=`max`, *sort*=`none`  - undefined
+  * *mode*=`min`, *sort*=`value` - ascending by value
+  * *mode*=`min`, *sort*=`index` - ascending by index
+  * *mode*=`min`, *sort*=`none`  - undefined
 
 **Example**
 

docs/ops/sort/TopK_3.md

@@ -10,7 +10,7 @@
 
 * *axis*
 
-  * **Description**: Specifies the axis along which 
+  * **Description**: Specifies the axis along which the values are retrieved.
   * **Range of values**: An integer. Negative value means counting dimension from the end.
   * **Type**: `int`
   * **Default value**: None
@@ -65,7 +65,15 @@ Output tensor is populated by values computes in the following way:
 
     output[i1, ..., i(axis-1), j, i(axis+1) ..., iN] = top_k(input[i1, ...., i(axis-1), :, i(axis+1), ..., iN]), k, sort, mode)
 
-So for each slice `input[i1, ...., i(axis-1), :, i(axis+1), ..., iN]` which represents 1D array, *TopK* value is computed individually. Sorting and minimum/maximum are controlled by `sort` and `mode` attributes.
+So for each slice `input[i1, ...., i(axis-1), :, i(axis+1), ..., iN]` which represents 1D array, *TopK* value is computed individually.
+
+Sorting and minimum/maximum are controlled by `sort` and `mode` attributes:
+  * *mode*=`max`, *sort*=`value` - descending by value
+  * *mode*=`max`, *sort*=`index` - ascending by index
+  * *mode*=`max`, *sort*=`none`  - undefined
+  * *mode*=`min`, *sort*=`value` - ascending by value
+  * *mode*=`min`, *sort*=`index` - ascending by index
+  * *mode*=`min`, *sort*=`none`  - undefined
 
 **Example**
 

inference-engine/tests/functional/plugin/cpu/shared_tests_instances/skip_tests_config.cpp

@@ -51,8 +51,7 @@ std::vector<std::string> disabledTestPatterns() {
         // TODO: Issue: 37862
         R"(.*ReverseSequenceLayerTest.*netPRC=(I8|U8).*)",
         // TODO: Issue: 38841
-        R"(.*TopKLayerTest.*k=10.*mode=min.*sort=index.*)",
+        R"(.*TopKLayerTest.*k=5.*sort=none.*)",
-        R"(.*TopKLayerTest.*k=5.*sort=(none|index).*)",
         // TODO: Issue: 43314
         R"(.*Broadcast.*mode=BIDIRECTIONAL.*inNPrec=BOOL.*)",
         // TODO: Issue 43417 sporadic issue, looks like an issue in test, reproducible only on Windows platform

ngraph/core/reference/include/ngraph/runtime/reference/topk.hpp

@@ -58,17 +58,10 @@ namespace ngraph
                 return a < b;
             }
 
-            template <typename T, typename U>
-            inline bool sort_indices_descending(const std::tuple<T, U>& a,
-                                                const std::tuple<T, U>& b)
-            {
-                return std::get<1>(a) < std::get<1>(b);
-            }
-
             template <typename T, typename U>
             inline bool sort_indices_ascending(const std::tuple<T, U>& a, const std::tuple<T, U>& b)
             {
-                return std::get<1>(a) > std::get<1>(b);
+                return std::get<1>(a) < std::get<1>(b);
             }
 
             template <typename T, typename U>
@@ -133,35 +126,18 @@ namespace ngraph
                                     compare_min<T, U>);
                     }
                     // Write temp vector to output
+                    switch (sort)
+                    {
+                    case op::v1::TopK::SortType::NONE: break;
+                    case op::v1::TopK::SortType::SORT_INDICES:
+                        std::sort(
+                            workspace.begin(), workspace.begin() + k, sort_indices_ascending<T, U>);
+                        break;
+                    case op::v1::TopK::SortType::SORT_VALUES:
                         if (compute_max)
-                    {
-                        switch (sort)
-                        {
-                        case op::v1::TopK::SortType::NONE: break;
-                        case op::v1::TopK::SortType::SORT_INDICES:
-                            std::sort(workspace.begin(),
-                                      workspace.begin() + k,
-                                      sort_indices_descending<T, U>);
-                            break;
-                        case op::v1::TopK::SortType::SORT_VALUES:
                             std::sort(workspace.begin(), workspace.begin() + k, compare_max<T, U>);
-                            break;
-                        }
-                    }
                         else
-                    {
-                        switch (sort)
-                        {
-                        case op::v1::TopK::SortType::NONE: break;
-                        case op::v1::TopK::SortType::SORT_INDICES:
-                            std::sort(workspace.begin(),
-                                      workspace.begin() + k,
-                                      sort_indices_ascending<T, U>);
-                            break;
-                        case op::v1::TopK::SortType::SORT_VALUES:
                             std::sort(workspace.begin(), workspace.begin() + k, compare_min<T, U>);
-                            break;
-                        }
                     }
                     for (size_t j = 0; j < k; j++)
                     {

ngraph/test/backend/topk.in.cpp

@@ -30,6 +30,8 @@
 #include "ngraph/runtime/tensor.hpp"
 #include "runtime/backend.hpp"
 #include "util/all_close_f.hpp"
+#include "util/engine/test_engines.hpp"
+#include "util/test_case.hpp"
 #include "util/test_control.hpp"
 #include "util/test_tools.hpp"
 
@@ -37,6 +39,7 @@ using namespace std;
 using namespace ngraph;
 
 static string s_manifest = "${MANIFEST}";
+using TestEngine = test::ENGINE_CLASS_NAME(${BACKEND_NAME});
 
 template <typename T>
 bool compare_set(const vector<T>& a, vector<T> b)
@@ -1265,3 +1268,74 @@ NGRAPH_TEST(${BACKEND_NAME}, topk_v1_invalid_k)
     const auto k_negative = op::Constant::create(element::i8, Shape{}, {-1});
     EXPECT_THROW(op::v1::TopK(data, k_negative, 0, "max", "index"), ngraph::NodeValidationFailure);
 }
+
+template <typename T>
+class topk_backend : public ::testing::Test
+{
+};
+TYPED_TEST_CASE_P(topk_backend);
+
+template <typename Mode, typename SortType>
+struct TopkSortTestOutputs
+{
+    Mode mode;
+    SortType sort_type;
+    std::vector<float> output0;
+    std::vector<int32_t> output1;
+
+    TopkSortTestOutputs(Mode mode,
+                        SortType sort_type,
+                        std::vector<float>&& output0,
+                        std::vector<int32_t>&& output1)
+        : mode(mode)
+        , sort_type(sort_type)
+        , output0(std::move(output0))
+        , output1(std::move(output1))
+    {
+    }
+};
+
+TYPED_TEST_P(topk_backend, topk_mode_sort_order)
+{
+    const Shape shape{5};
+    const Shape rshape{3};
+    const auto data = make_shared<op::Parameter>(element::f32, shape);
+    const auto k = op::Constant::create(element::i64, {}, {3});
+    const int64_t axis = 0;
+
+    // helpers to reduce code verbosity
+    using m = typename TypeParam::Mode;
+    using st = typename TypeParam::SortType;
+    using v_f = std::vector<float>;
+    using v_i = std::vector<int32_t>;
+
+    const std::vector<float> input{3, 1, 2, 5, 4};
+
+    std::vector<TopkSortTestOutputs<m, st>> valid_outputs;
+    valid_outputs.emplace_back(m::MAX, st::SORT_VALUES, v_f{5, 4, 3}, v_i{3, 4, 0});
+    valid_outputs.emplace_back(m::MAX, st::SORT_INDICES, v_f{3, 5, 4}, v_i{0, 3, 4});
+    valid_outputs.emplace_back(m::MIN, st::SORT_VALUES, v_f{1, 2, 3}, v_i{1, 2, 0});
+    valid_outputs.emplace_back(m::MIN, st::SORT_INDICES, v_f{3, 1, 2}, v_i{0, 1, 2});
+
+    for (const auto& v : valid_outputs)
+    {
+        auto topk = make_shared<TypeParam>(data, k, axis, v.mode, v.sort_type);
+        auto f0 = make_shared<Function>(OutputVector{topk->output(0)}, ParameterVector{data});
+        auto f1 = make_shared<Function>(OutputVector{topk->output(1)}, ParameterVector{data});
+
+        auto test_case0 = test::TestCase<TestEngine>(f0);
+        test_case0.add_input<float>(input);
+        test_case0.add_expected_output<float>(rshape, v.output0);
+        test_case0.run();
+
+        auto test_case1 = test::TestCase<TestEngine>(f1);
+        test_case1.add_input<float>(input);
+        test_case1.add_expected_output<int32_t>(rshape, v.output1);
+        test_case1.run();
+    }
+}
+
+REGISTER_TYPED_TEST_CASE_P(topk_backend, topk_mode_sort_order);
+
+typedef ::testing::Types<op::v1::TopK, op::v3::TopK> TopKTypes;
+INSTANTIATE_TYPED_TEST_CASE_P(${BACKEND_NAME}, topk_backend, TopKTypes);