[CPU]: Added impl of SearchSorted op #27036

pkowalc1 · 2024-10-14T14:39:52Z

Details:

Added support for SearchSorted op with tests.

Tickets:

CVS-154858

src/plugins/intel_cpu/src/nodes/search_sorted.cpp

dmitry-gorokhov · 2024-10-17T12:17:40Z

src/tests/functional/shared_test_classes/src/single_op/search_sorted.cpp

+    ElementType inputPrecision;
+    std::tie(searchSortedParams, inputPrecision) = basicParamsSet;
+
+    targetDevice = ov::test::utils::DEVICE_CPU;


Since it is shared test targetDevice should be test parameter.

dmitry-gorokhov · 2024-10-17T13:15:08Z

src/plugins/intel_cpu/src/nodes/search_sorted.cpp

+
+    SearchSortedContext ctx = {*this};
+
+#define CASE(OV_TYPE) OV_CASE2(OV_TYPE, ov::element::i64, ov::element_type_traits<OV_TYPE>::value_type, int64_t)


OV CPU Plugin doesn't support i64 data_type for internal tensor. So current tests work only because SearchSorted op is last op in the graph: SearchSorted [i64] -> Result.
In case there is other op after SearchSorted than precision will be: SearchSorted [i32] -> Op -> [i32] -> Convert [i64] -> Result.
So if we still need to support i64 for the first case lets handle i32 in switch below as well. Otherwise lets leave i32 only.

i64 is needed as some models use it, so fixed by adding i32.

nshchego · 2024-10-17T22:49:35Z

...tests/functional/shared_test_classes/include/shared_test_classes/single_op/search_sorted.hpp

+    void generate_inputs(const std::vector<ov::Shape>& targetInputStaticShapes) override;
+};
+
+extern const std::vector<SearchSortedSpecificParams> SearchSortedParamsVector;


Where is it used?

nshchego · 2024-10-17T22:50:23Z

src/tests/functional/shared_test_classes/src/single_op/search_sorted.cpp

+
+static const int SEED = 7877;
+
+std::string SearchSortedLayerTest::getTestCaseName(testing::TestParamInfo<SearchSortedLayerTestParams> obj) {


std::string SearchSortedLayerTest::getTestCaseName(const testing::TestParamInfo<SearchSortedLayerTestParams>& obj) {

nshchego · 2024-10-17T22:54:59Z

src/tests/functional/shared_test_classes/src/single_op/search_sorted.cpp

+
+    ElementType inputPrecision;
+    std::string targetDevice;
+    std::tie(searchSortedParams, inputPrecision, targetDevice) = basicParamsSet;


Better to use reference to avoid extra copy const auto& p = std::get<n>(basicParamsSet)

Every single test on CPU side I have checked is doing it this way - so will keep it to be that way to be consistent.

nshchego · 2024-10-17T23:06:20Z

src/tests/functional/shared_test_classes/src/single_op/search_sorted.cpp

+    inputs.insert({funcInputs[0].get_node_shared_ptr(), sortedTensor});
+
+    auto valuesTensor =
+        ov::test::utils::create_and_fill_tensor_unique_sequence(dataPrecision, targetInputStaticShapes[1], 0, 8, SEED);


There is no statement in specification that the second input should be sorted and unique.

Good point - fixed.

nshchego · 2024-10-17T23:12:00Z

...ugins/intel_cpu/tests/functional/shared_tests_instances/single_layer_tests/search_sorted.cpp

+INSTANTIATE_TEST_SUITE_P(smoke_SearchSortedTest,
+                         SearchSortedLayerTest,
+                         ::testing::Combine(::testing::ValuesIn(SearchSortedLayerTest::GenerateParams()),
+                                            testing::Values(ElementType::f32, ElementType::f16),


Please add i8 at least

I have added tests for i64 and u32 - since they are used in the model that needs SearchSorted.

I cannot add currently test for i8 due to a bug in data_utils.hpp:317, which causes generate_input to loop infinitively. After fixing it, it will work, but fixing that bug is orthogonal to this PR and should be done in separate PR.

nshchego · 2024-10-17T23:17:41Z

src/tests/functional/shared_test_classes/src/single_op/search_sorted.cpp

+
+const std::vector<SearchSortedSpecificParams> SearchSortedLayerTest::GenerateParams() {
+    const std::vector<SearchSortedSpecificParams> params = {
+        SearchSortedSpecificParams{InputShape{{}, {{1, 18, 104}}}, InputShape{{}, {{1, 18, 104}}}, true},


Please add more shapes instances to check dynamic case. Like {{}, {{1, 18, 104}, {3, 50, 70}, ...}}

Added more tests cases.

dmitry-gorokhov · 2024-10-18T10:16:29Z

src/plugins/intel_cpu/src/nodes/search_sorted.cpp

+
+#define CASE(OV_TYPE)                                                                           \
+    OV_CASE2(OV_TYPE, ov::element::i64, ov::element_type_traits<OV_TYPE>::value_type, int64_t), \
+        OV_CASE2(OV_TYPE, ov::element::i32, ov::element_type_traits<OV_TYPE>::value_type, int32_t)


Please fix alignment

dmitry-gorokhov · 2024-10-18T10:17:28Z

src/plugins/intel_cpu/src/nodes/search_sorted.cpp

+              CASE(ov::element::f16),
+              CASE(ov::element::bf16),
+              CASE(ov::element::i32),
+              CASE(ov::element::u32),


U32 is not needed since plugin converts it to i32

dmitry-gorokhov · 2024-10-18T10:38:36Z

src/plugins/intel_cpu/src/nodes/search_sorted.cpp

+    OV_CASE2(OV_TYPE, ov::element::i64, ov::element_type_traits<OV_TYPE>::value_type, int64_t), \
+        OV_CASE2(OV_TYPE, ov::element::i32, ov::element_type_traits<OV_TYPE>::value_type, int32_t)
+
+    OV_SWITCH(intel_cpu,


Since OV_SWITCH doesn't check if any condition is satisfied, we need to guarantee inputPrecision and outputPrecision are in supported range. In order to do that lets adjust precision which we use in initSupportedPrimitiveDescriptors. See as as example https://github.com/openvinotoolkit/openvino/blob/master/src/plugins/intel_cpu/src/nodes/scatter_update.cpp#L273-L283.

Fixed - as discussed in separate channel.

[WIP]: [CPU]: SearchSorted added impl and stub for tests.

f721e7f

pkowalc1 added the WIP work in progress label Oct 14, 2024

pkowalc1 requested review from a team as code owners October 14, 2024 14:39

github-actions bot added the category: CPU OpenVINO CPU plugin label Oct 14, 2024

pkowalc1 added 2 commits October 15, 2024 18:26

[CPUPlugin]: SearchSorted: Fixes.

dc4b384

[func tests]: Fixed CPU SearchSorted op test.

ea10ba8

pkowalc1 mentioned this pull request Oct 16, 2024

[Pytorch fronted]: Added support for Search Sorted op #26976

Open

pkowalc1 added 2 commits October 16, 2024 10:51

[CPUPlugin func test]: Refactoring.

d2e0117

[CPUFuncTest]: Refactoring.

8b95ffa

pkowalc1 requested review from a team as code owners October 16, 2024 12:44

github-actions bot added the category: IE Tests OpenVINO Test: plugins and common label Oct 16, 2024

[CPUFuncTest]: SearchSorted: Refactoring and added more tests.

36026bd

pkowalc1 removed the WIP work in progress label Oct 16, 2024

pkowalc1 changed the title ~~[WIP]: [CPU]: SearchSorted added impl.~~ [CPU]: SearchSorted added impl. Oct 16, 2024

pkowalc1 changed the title ~~[CPU]: SearchSorted added impl.~~ [CPU]: Added impl of SearchSorted op Oct 16, 2024

mlukasze assigned nshchego Oct 17, 2024

mlukasze requested a review from dmitry-gorokhov October 17, 2024 04:37

dmitry-gorokhov reviewed Oct 17, 2024

View reviewed changes

dmitry-gorokhov added this to the 2024.5 milestone Oct 17, 2024

[CPUPlugin]: SearchSorted: review fixes.

88e2a94

nshchego reviewed Oct 17, 2024

View reviewed changes

pkowalc1 added 2 commits October 18, 2024 07:19

[CPU Plugin]: Review fixes.

17a8faf

[cpu func test]: Fixed SearchSorted CPU test.

2d7e17b

[CPU func tests]: Added more test cases for SearchSorted.

3114067

dmitry-gorokhov reviewed Oct 18, 2024

View reviewed changes

pkowalc1 added 2 commits October 18, 2024 11:35

[CPU]: SearchSorted: fixes due to review.

96a4668

[CPU]: SearchSorted: Review fixes.

e126353

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CPU]: Added impl of SearchSorted op #27036

[CPU]: Added impl of SearchSorted op #27036

pkowalc1 commented Oct 14, 2024 •

edited

Loading

dmitry-gorokhov Oct 17, 2024

pkowalc1 Oct 17, 2024

dmitry-gorokhov Oct 17, 2024

pkowalc1 Oct 17, 2024

nshchego Oct 17, 2024

pkowalc1 Oct 18, 2024

nshchego Oct 17, 2024

pkowalc1 Oct 18, 2024

nshchego Oct 17, 2024

pkowalc1 Oct 18, 2024

nshchego Oct 17, 2024

pkowalc1 Oct 18, 2024

nshchego Oct 17, 2024

pkowalc1 Oct 18, 2024 •

edited

Loading

nshchego Oct 17, 2024

pkowalc1 Oct 18, 2024

dmitry-gorokhov Oct 18, 2024

pkowalc1 Oct 18, 2024

dmitry-gorokhov Oct 18, 2024

pkowalc1 Oct 18, 2024

dmitry-gorokhov Oct 18, 2024

pkowalc1 Oct 18, 2024


		SearchSortedContext ctx = {*this};

		#define CASE(OV_TYPE) OV_CASE2(OV_TYPE, ov::element::i64, ov::element_type_traits<OV_TYPE>::value_type, int64_t)


		static const int SEED = 7877;

		std::string SearchSortedLayerTest::getTestCaseName(testing::TestParamInfo<SearchSortedLayerTestParams> obj) {

[CPU]: Added impl of SearchSorted op #27036

Are you sure you want to change the base?

[CPU]: Added impl of SearchSorted op #27036

Conversation

pkowalc1 commented Oct 14, 2024 • edited Loading

Details:

Tickets:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pkowalc1 Oct 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pkowalc1 commented Oct 14, 2024 •

edited

Loading

pkowalc1 Oct 18, 2024 •

edited

Loading