【PaddlePaddle Hackathon 3】Add Paddle sum operator #12545

Patrick-Star125 · 2022-08-13T05:34:09Z

Details:

add sum operation in Paddle front end

Reference:

Unit-test passed

Patrick-Star125 · 2022-08-13T05:53:01Z

I faced this compilation problem and I don't know how to fix it.

It says my implementation includes the illegal conversion from list to <ov::Node>

But the sum in my code should always be type of Output<Node> logically,and I'm not sure whether my usage of function Add caused this error.

openvino-dev-samples · 2022-08-15T07:21:01Z

I faced this compilation problem and I don't know how to fix it. It says my implementation includes the illegal conversion from list to <ov::Node> But the sum in my code should always be type of Output<Node> logically,and I'm not sure whether my usage of function Add caused this error.

Hi Did you check the type of data[0] ? maybe it is a list ?

openvino-dev-samples

Hi Thanks for your contribution

openvino-dev-samples · 2022-08-15T07:23:01Z

src/core/tests/frontend/paddle/test_models/gen_scripts/generate_sum.py

+
+
+def sum(name: str, x, dtype=None):
+    out = paddle.add_n(x)


Hi
According to your reference document: https://www.paddlepaddle.org.cn/documentation/docs/zh/1.8/api_cn/layers_cn/sum_cn.html#sum
seems we should use paddle.fluid.layers.sum(x) to simulate paddle's output right?

Patrick-Star125 · 2022-08-17T08:35:07Z

@OpenVINO-dev-contest Hi, I ran into another problem. Exception shows as blow

I don't know where I implement this 'paddle.fluid.core_avx.BlockDesc' object. Can you help check my unit test code?
Maybe API fill_contant() is the error source
I need input a Python list consist of Paddle Tensor into sum Op, Is that can be implemented directly?

openvino-dev-samples · 2022-08-18T07:13:50Z

src/core/tests/frontend/paddle/test_models/gen_scripts/generate_sum.py

+
+def sum(name: str, x, dtype=None):
+    out = fluid.layers.sum(x)
+    exe = paddle.fluid.Executor(paddle.fluid.core.CPUPlace())


hi I think you have to convert x to a paddle.static.data object before feed into sum(), pls refer to other generation script.

Patrick-Star125 · 2022-08-18T12:53:47Z

Thanks, problem fixed. Now can you pls review my code? @OpenVINO-dev-contest

openvino-dev-samples · 2022-08-22T04:01:49Z

src/frontends/paddle/src/op/sum.cpp

+        sum = std::make_shared<default_opset::Add>(sum, data[i]);
+    }
+    const auto temp = default_opset::Constant::create(input_type, {shape}, {0});
+    return node.default_single_output_mapping({std::make_shared<default_opset::Add>(sum, temp)}, {"Out"});


Hi @Patrick-Star125 can we reuse the 'sum' here, not create a constant to build a new node again ?

I thought so at first.
But if I reuse the sum, it will cause a error in the compile phase.

Maybe we can create a 'Constant Node' to initialize the sum node ? but not initialize it by data[0] ? I guess.

I thought so at first, so I tried to implement it as below.

But it will cause another error in the compile phase.

I thought so at first, so I tried to implement it as below. But it will cause another error in the compile phase.

Got it. Thanks. How about leaving the {std::make_shared<default_opset::Add>(sum, temp)} before the loop, to transfer the first input element into a Add node. And let's see other reviewer's comments.

Thanks, the problem was solved by initializing the sum with a copied pointer of the data object.

src/frontends/paddle/src/op/sum.cpp

openvino-dev-samples · 2022-08-22T04:07:07Z

src/frontends/paddle/src/op/sum.cpp

+NamedOutputs sum(const NodeContext& node) {
+    auto data = node.get_ng_inputs("X");
+    const auto input_type = data[0].get_element_type();
+    const auto shape = data[0].get_shape();


Shall we check the shape of each element in list to make sure they are same ?
You can leverage 'PADDLE_OP_CHECK' to log a message if the shapes are not aligned.

src/core/tests/frontend/paddle/test_models/gen_scripts/generate_sum.py

ilyachur

@ceciliapeng2011 Looks like we have the same op in the PR: #12696

openvino-dev-samples · 2022-09-01T03:53:46Z

@ceciliapeng2011 Looks like we have the same op in the PR: #12696

hi @ilyachur, since this PR is submitted before another one, could you help to review it with higher priority if it is almost qualified ? thanks

src/core/tests/frontend/paddle/test_models/gen_scripts/generate_sum.py

src/frontends/paddle/src/op/sum.cpp

liubo-intel

Hi, @Patrick-Star125 : thanks for contribution of this Paddle OP conversion. have left some comments mainly related with that I think we would better keep align with PaddlePaddle2.1 'sum' Operator, as 'PaddlePaddle2.1' is openvino currently officially supported version .
And also modified the 'Reference' link of this PR's description(changed spec version from 1.8 to 2.1, and the language version from Chinese to English, convenient for global viewers)

src/core/tests/frontend/paddle/test_models/gen_scripts/generate_sum.py

src/frontends/paddle/src/op/sum.cpp

src/core/tests/frontend/paddle/test_models/gen_scripts/generate_sum.py

ilyachur · 2022-09-13T12:04:04Z

src/frontends/paddle/src/op/sum.cpp

+NamedOutputs sum(const NodeContext& node) {
+    auto data = node.get_ng_inputs("X");
+    const auto input_type = data[0].get_element_type();
+    const auto shape = data[0].get_shape();


I don't see that you use shape and input_type here. I think we should remove these variables

Done, removed these redundant codes already.

Patrick-Star125 requested a review from a team as a code owner August 13, 2022 05:34

Patrick-Star125 mentioned this pull request Aug 13, 2022

【PaddlePaddle Hackathon 第三期】任务总览 PaddlePaddle/Paddle#43938

Closed

openvino-dev-samples reviewed Aug 15, 2022

View reviewed changes

openvino-dev-samples reviewed Aug 18, 2022

View reviewed changes

openvino-dev-samples reviewed Aug 22, 2022

View reviewed changes

src/core/tests/frontend/paddle/test_models/gen_scripts/generate_sum.py Outdated Show resolved Hide resolved

add paddle op sum

13cc0c9

Patrick-Star125 requested a review from openvino-dev-samples August 23, 2022 03:28

Patrick-Star125 added 2 commits August 23, 2022 11:34

new version

89c62e1

Merge remote-tracking branch 'open/master' into sum

2b70d0b

yuxu42 requested review from liubo-intel and removed request for openvino-dev-samples August 25, 2022 02:51

ilyachur added this to the 2022.3 milestone Sep 1, 2022

ilyachur added the category: PDPD FE OpenVINO PaddlePaddle FrontEnd label Sep 1, 2022

ilyachur reviewed Sep 1, 2022

View reviewed changes

ceciliapeng2011 added the PaddlePaddle Hackathon a Intel and Baidu joint Hackathon event label Sep 1, 2022

ceciliapeng2011 requested changes Sep 1, 2022

View reviewed changes

src/core/tests/frontend/paddle/test_models/gen_scripts/generate_sum.py Outdated Show resolved Hide resolved

src/core/tests/frontend/paddle/test_models/gen_scripts/generate_sum.py Outdated Show resolved Hide resolved

src/frontends/paddle/src/op/sum.cpp Outdated Show resolved Hide resolved

liubo-intel reviewed Sep 1, 2022

View reviewed changes

src/core/tests/frontend/paddle/test_models/gen_scripts/generate_sum.py Outdated Show resolved Hide resolved

src/frontends/paddle/src/op/sum.cpp Show resolved Hide resolved

Merge remote-tracking branch 'open/master' into sum

a534127

Patrick-Star125 requested a review from a team as a code owner September 1, 2022 08:09

add unit test for other precision

61b9d9e

liubo-intel approved these changes Sep 1, 2022

View reviewed changes

src/core/tests/frontend/paddle/test_models/gen_scripts/generate_sum.py Outdated Show resolved Hide resolved

Patrick-Star125 and others added 3 commits September 2, 2022 09:08

Merge remote-tracking branch 'open/master' into sum

3a415e5

change layer.sum to add_n

1180792

Merge branch 'master' into sum

47345bb

liubo-intel requested a review from ceciliapeng2011 September 9, 2022 00:33

Merge branch 'master' into sum

90e183f

ceciliapeng2011 approved these changes Sep 13, 2022

View reviewed changes

ceciliapeng2011 requested a review from ilyachur September 13, 2022 09:10

ilyachur reviewed Sep 13, 2022

View reviewed changes

Patrick-Star125 added 2 commits September 13, 2022 22:59

Merge remote-tracking branch 'open/master' into sum

86ff2d7

remove redundant code

b4b2e3a

ilyachur approved these changes Sep 14, 2022

View reviewed changes

ilyachur enabled auto-merge (squash) September 14, 2022 03:26

Merge branch 'master' into sum

b188b4c

ilyachur merged commit 8cfb285 into openvinotoolkit:master Sep 15, 2022

ilya-lavrenov added the ExternalPR External contributor label Mar 4, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

【PaddlePaddle Hackathon 3】Add Paddle sum operator #12545

【PaddlePaddle Hackathon 3】Add Paddle sum operator #12545

Patrick-Star125 commented Aug 13, 2022 •

edited by liubo-intel

Loading

Patrick-Star125 commented Aug 13, 2022 •

edited

Loading

openvino-dev-samples commented Aug 15, 2022

openvino-dev-samples left a comment

openvino-dev-samples Aug 15, 2022

Patrick-Star125 Aug 17, 2022

Patrick-Star125 commented Aug 17, 2022

openvino-dev-samples Aug 18, 2022

Patrick-Star125 commented Aug 18, 2022 •

edited

Loading

openvino-dev-samples Aug 22, 2022

Patrick-Star125 Aug 22, 2022

openvino-dev-samples Aug 22, 2022

Patrick-Star125 Aug 22, 2022

openvino-dev-samples Aug 22, 2022

Patrick-Star125 Aug 23, 2022

openvino-dev-samples Aug 22, 2022

Patrick-Star125 Aug 22, 2022

ilyachur left a comment

openvino-dev-samples commented Sep 1, 2022 •

edited

Loading

liubo-intel left a comment

ilyachur Sep 13, 2022

Patrick-Star125 Sep 13, 2022 •

edited

Loading

【PaddlePaddle Hackathon 3】Add Paddle sum operator #12545

【PaddlePaddle Hackathon 3】Add Paddle sum operator #12545

Conversation

Patrick-Star125 commented Aug 13, 2022 • edited by liubo-intel Loading

Details:

Reference:

Unit-test passed

Patrick-Star125 commented Aug 13, 2022 • edited Loading

openvino-dev-samples commented Aug 15, 2022

openvino-dev-samples left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Patrick-Star125 commented Aug 17, 2022

Choose a reason for hiding this comment

Patrick-Star125 commented Aug 18, 2022 • 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

ilyachur left a comment

Choose a reason for hiding this comment

openvino-dev-samples commented Sep 1, 2022 • edited Loading

liubo-intel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Patrick-Star125 Sep 13, 2022 • edited Loading

Choose a reason for hiding this comment

Patrick-Star125 commented Aug 13, 2022 •

edited by liubo-intel

Loading

Patrick-Star125 commented Aug 13, 2022 •

edited

Loading

Patrick-Star125 commented Aug 18, 2022 •

edited

Loading

openvino-dev-samples commented Sep 1, 2022 •

edited

Loading

Patrick-Star125 Sep 13, 2022 •

edited

Loading