-
Notifications
You must be signed in to change notification settings - Fork 5.7k
refine pybind when *_op.cc contains several operators #7148
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
Conversation
paddle/operators/CMakeLists.txt
Outdated
# It's enough to just adding one operator to pybind | ||
file(APPEND ${pybind_file} "USE_OP(reduce_sum);\n") | ||
endif() | ||
# net_op doesn't need pybind, others will be pybind manually |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line of comment can be:
Define operators that don't need pybind here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Thank you.
paddle/operators/CMakeLists.txt
Outdated
string(REGEX MATCH "REGISTER_OP\\(.*REGISTER_OP\\(" multi_register "${TARGET_CONTENT}") | ||
string(REGEX MATCH "REGISTER_OP\\([a-z0-9_]*," one_register "${multi_register}") | ||
if (one_register STREQUAL "") | ||
string(REPLACE "_op" "" TARGET "${TARGET}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it necessary to add some debug message here to see the registered operators?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As there are hundreds of operators, the registered message are a lot too. If we want to see them, we can refer to paddle/pybind/pybind.h
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM++
hi @luotao1, just to confirm the reason for "it's enough to just adding one operator to pybind in a *_op.cc file" is that the linking of a single operator causes the linking of the whole object file, therefore all operators in that object file will be registered. |
fix #7059 (comment)
conv_op
,conv_cudnn_op
, etcREGEX MATCH REGISTER_OP(***,
, where***
is the name forUSE_OP
/USE_CPU_ONLY_OP
etc .REGISTER_OP
is manually, such ascompare_op
:Paddle/paddle/operators/compare_op.cc
Lines 81 to 105 in fba6a10