Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

【PaddlePaddle Hackathon 3】No.102:为 OpenVINO 实现 Paddle 算子 elementwise_floordiv 转换 #12186

Conversation

Copy link
Contributor

@openvino-dev-samples openvino-dev-samples left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution

src/frontends/paddle/src/op/elementwise_ops.cpp Outdated Show resolved Hide resolved
@openvino-dev-samples
Copy link
Contributor

@meiyang-intel Could you help to take a look ? thanks

if ((axis == -1) || (axis == x_rank - 1) || (x_rank == y_rank)) {
return node_context.default_single_output_mapping({std::make_shared<default_opset::Floor>(std::make_shared<default_opset::Divide>(x, y))}, {"Out"});
} else {
std::vector<int64_t> indices;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use broadcast to address the case when x and y have different rank?

Copy link
Author

@glare-ni glare-ni Sep 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里参考的是elementwise_ops原始代码中的实现,里面手动进行brodcast的,没看到自动broadcast的例子,有参考实现代码吗?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please develop such kind of test cases. And apply openvino AutoBroadcastSpec to the op mapper.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test case后面添加。
能否给个调用的例子参考呀,或者文档链接来着?
比如,实现” x + y“的调用例子,x与y不同维度,且y需要brodcast的例子?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please comment in English. The reviewers from abroad. Thanks.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi, i found an error in paddlepaddle floor_divide.
if value is a negative number eg."-0.2", paddle will use "ceil", the value will be "0"
if value is a positive number eg "1.2", paddle will user "floor" the value will be "1".
this is a demo.

import numpy as np

x = np.array([1, 2, -3])
y = np.array([-2, 1, 2])

np.floor_divide(x, y)
# [-1, 2, -2]
import paddle
paddle.set_device("cpu")

x = paddle.to_tensor([1, 2, -3])
y = paddle.to_tensor([-2, 1, 2])

paddle.floor_divide(x, y)
# [0, 2, -1]

paddle.floor( x.astype("float32") / y.astype("float32")).astype("int65")
# [-1, 2, -2]

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i open a issue in paddlepaddle PaddlePaddle/Paddle#46379

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

default_opset::Divide is equal "numpy floor divide".

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not ceil and floor , actualy use trunc

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great finding! Thanks for this. We will collaborate with Paddle to see how to resolve this conflict.

@openvino-dev-samples
Copy link
Contributor

Since no feedback from contributor for 2 weeks, this PR will be pending for review.

@glare-ni glare-ni requested a review from a team as a code owner September 15, 2022 12:58
@glare-ni
Copy link
Author

doing

@glare-ni
Copy link
Author

293e6a9c80bbeae1362ab9a396d8122

done must use floor(divide(x, y))

@glare-ni glare-ni requested review from ceciliapeng2011 and removed request for meiyang-intel September 22, 2022 02:31
@akladiev
Copy link
Collaborator

This PR will be closed in 2 weeks in case of no activity.

@akladiev akladiev added the Stale label May 11, 2023
@akladiev
Copy link
Collaborator

This PR was closed because it has been stalled for 2 week with no activity.

@akladiev akladiev closed this May 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants