Skip to content

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

Merged
merged 4 commits into from
Jan 3, 2018
Merged

refine pybind when *_op.cc contains several operators #7148

merged 4 commits into from
Jan 3, 2018

Conversation

luotao1
Copy link
Contributor

@luotao1 luotao1 commented Jan 2, 2018

fix #7059 (comment)

  • for most op contains several operators, such as conv_op, conv_cudnn_op, etc
    • auto pybind it by REGEX MATCH REGISTER_OP(***,, where *** is the name for USE_OP/USE_CPU_ONLY_OP etc .
  • for other op whose REGISTER_OP is manually, such as compare_op:
    #define REGISTER_LOGICAL_OP(op_type, _equation) \
    struct _##op_type##Comment { \
    static char type[]; \
    static char equation[]; \
    }; \
    char _##op_type##Comment::type[]{#op_type}; \
    char _##op_type##Comment::equation[]{_equation}; \
    REGISTER_OPERATOR( \
    op_type, ::paddle::operators::CompareOp, \
    ::paddle::operators::CompareOpProtoMaker<_##op_type##Comment>, \
    ::paddle::operators::CompareOpInferShape<_##op_type##Comment>, \
    ::paddle::framework::EmptyGradOpMaker);
    REGISTER_LOGICAL_OP(less_than, "Out = X < Y");
    REGISTER_LOGICAL_KERNEL(less_than, CPU, paddle::operators::LessThanFunctor);
    REGISTER_LOGICAL_OP(less_equal, "Out = X <= Y");
    REGISTER_LOGICAL_KERNEL(less_equal, CPU, paddle::operators::LessEqualFunctor);
    REGISTER_LOGICAL_OP(greater_than, "Out = X > Y");
    REGISTER_LOGICAL_KERNEL(greater_than, CPU,
    paddle::operators::GreaterThanFunctor);
    REGISTER_LOGICAL_OP(greater_equal, "Out = X >= Y");
    REGISTER_LOGICAL_KERNEL(greater_equal, CPU,
    paddle::operators::GreaterEqualFunctor);
    REGISTER_LOGICAL_OP(equal, "Out = X == Y");
    REGISTER_LOGICAL_KERNEL(equal, CPU, paddle::operators::EqualFunctor);
    • manually pybind them.

@luotao1 luotao1 requested a review from typhoonzero January 2, 2018 14:29
# 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
Copy link
Contributor

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.

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. Thank you.

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}")
Copy link
Contributor

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?

Copy link
Contributor Author

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.

Copy link
Contributor

@typhoonzero typhoonzero left a comment

Choose a reason for hiding this comment

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

LGTM++

@luotao1 luotao1 merged commit 231e2ee into PaddlePaddle:develop Jan 3, 2018
@luotao1 luotao1 deleted the op_make branch January 3, 2018 12:57
@tonyyang-svail
Copy link

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.

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.

need to refine CMakeLists.txt of paddle/operators
3 participants