Meiyang/fix cvs58539 #8217
Meiyang/fix cvs58539 #8217
Conversation
meiyang-intel
force-pushed
the
meiyang/fix_CVS58539
branch
2 times, most recently
from
27e826b to
ca8eaa0
Compare
meiyang-intel
force-pushed
the
meiyang/fix_CVS58539
branch
from
205e394 to
40953ed
Compare
meiyang-intel
force-pushed
the
meiyang/fix_CVS58539
branch
from
da765ca to
a6bd118
Compare
|
Also another concern is about the performance, do we have test this branch in benchmark app to see if the performance is effected ? |
| else { | ||
| auto axis = ngraph::opset6::Constant::create(ngraph::element::i64, ov::Shape{}, {axis_value}); | ||
|
|
||
| auto x_shape = std::make_shared<ngraph::opset6::ShapeOf>(x); |
There was a problem hiding this comment.
We'd better to use the default_opset instead of opset6, the sample
meiyang-intel
force-pushed
the
meiyang/fix_CVS58539
branch
from
a6bd118 to
dd2122a
Compare
|
Hi @jane-intel , can you help review this PR |
| hw_end, | ||
| std::vector<int64_t>{0}, | ||
| std::vector<int64_t>{0}); | ||
| const std::vector<size_t> hw_indices{1, 2, 3}; |
There was a problem hiding this comment.
2D conv has NCHW layout of data and OIHW on weight, right? Why do we state axes 1,2,3 as spatial? should we change naming and add a comment?
There was a problem hiding this comment.
@jane-intel , it is updated
|
|
||
| auto indices_node = | ||
| default_opset::Constant::create(ngraph::element::i64, ngraph::Shape{indices.size()}, indices); | ||
| auto y_node = std::make_shared<default_opset::Unsqueeze>(y, indices_node); | ||
| return node.default_single_output_mapping({std::make_shared<T>(x, y_node)}, {"Out"}); |
There was a problem hiding this comment.
why can't we set auto broadcast to PDPD mode and skip this Unsqueeze? Will it be supported by plugins and further tools? I see this as proper way to read eltwise ops
There was a problem hiding this comment.
Hi @jane-intel , updated by using PDPD broadcast. also find some bugs in transforming while testing. please check
There was a problem hiding this comment.
Hi @jane-intel , I also find a bug in transformer if we apply PDPD autobroadcast. There is a workaround to skip this issue. The workaround is in https://github.com/meiyang-intel/openvino/tree/meiyang/op_sequence_fusion_rm_pdpd_broadcast . But this WA will reduce the performance some paddle models. And the fix to apply broadcast to op sequence fusion is in progress.
But after second consideration, these fix for transformer may not only impact paddle. Also the 2022.1 is upcoming, so I don't want to import these changes to 2022.1 to prevent from unknown issues.
So can I revert the commit which is applied the PDPD broadcast and using 'unsqueeze' in 2022.1?
There was a problem hiding this comment.
Okay, proceed with unsqueeze. I was simply trying to understand if pdpd broadcast for eltwise ops could be a solution here.
meiyang-intel
requested review from
GlebKazantaev and
ilyachur
as code owners
meiyang-intel
force-pushed
the
meiyang/fix_CVS58539
branch
2 times, most recently
from
7f1647c to
fc8d57e
Compare
This reverts commit ff552d2efe47286910df9876e1b0d97ff6301695.
This reverts commit b2e16633c192bddfb94963465d175f98fdad1719.
This reverts commit a30a93f5b80f0a88064112e498f83eb9b22aa4bd. use general opset in paddle elementwise op
meiyang-intel
force-pushed
the
meiyang/fix_CVS58539
branch
from
a98b06c to
b9cd82c
Compare
Details:
Tickets: