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

elementwise_mul refactor #37471

Merged
merged 9 commits into from
Nov 24, 2021

Conversation

YuanRisheng
Copy link
Contributor

PR types

Others

PR changes

OPs

Describe

Refactor elementwise_mul kernel and slim elementwise code.

@paddle-bot-old
Copy link

Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

zyfncg
zyfncg previously approved these changes Nov 24, 2021
// Create the definition of ElementwiseMul
DEFINE_ELEMENTWISE_OP(Mul)
// Create the definition of ElementwiseDiv
DEFINE_ELEMENTWISE_OP(Div)
Copy link
Contributor

Choose a reason for hiding this comment

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

在CPU的math.cc里没定义Div,这里是多写了吗?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

没有多写,CUDA下的div函数删掉了,取而代之用这个div,cpu下的以前的div没删掉,所以没写

@@ -228,4 +228,29 @@ struct SameDimsElementwiseCompute {
}
};

#define DEFINE_ELEMENTWISE_OP(name) \
Copy link
Contributor

Choose a reason for hiding this comment

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

如果一定要用这种形式缩减代码,建议将宏定义放到kernels/cpu/**.h中,functions中的内容定位是由kernel来调用的,反向决定kernel的写法是不合适的

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

ASSERT_NEAR(expect_result[0][0], actual_result0, 1e-6f);
ASSERT_NEAR(expect_result[0][1], actual_result1, 1e-6f);
ASSERT_NEAR(expect_result[1][0], actual_result2, 1e-6f);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

后面建议命名空间都规范下,参照PR #37456

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

@chenwhql chenwhql left a comment

Choose a reason for hiding this comment

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

LGTM

@chenwhql chenwhql merged commit c5e857d into PaddlePaddle:develop Nov 24, 2021
Zjq9409 pushed a commit to Zjq9409/Paddle that referenced this pull request Dec 10, 2021
* elementwise_mul refactor

* perfect code in test

* delete redundant code

* fix bugs when run test_multiply

* adjust the location of macro

* fix bugs when run ci
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants