Add a sample op, add_op
#2827
Merged
Add a sample op, add_op
#2827
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
reyoung
force-pushed
the
feature/op_kernel_add
branch
from
84ac1af to
ab48ef8
Compare
QiJune
reviewed
Jul 13, 2017
paddle/op/add_op.h
Outdated
There was a problem hiding this comment.
这里的模板参数应该有两个吧，一个是设备类型，一个是数据类型
template <typename DeviceType, typename DataType>
class AddKernel
There was a problem hiding this comment.
目前既然没有不同数据类型的实现，就先不考虑不同数据类型的问题。
paddle/framework/op_registry.h
Outdated
There was a problem hiding this comment.
目前既然没有不同数据类型的实现，就先不考虑不同数据类型的问题。
There was a problem hiding this comment.
会有double、float、half等数据类型的支持吧，希望从接口下考虑下。
There was a problem hiding this comment.
如果需要增加数据类型的话，只是注册的时候加一个参数就好了。除了注册之外的接口没有区别。
reyoung
force-pushed
the
feature/op_kernel_add
branch
from
ab48ef8 to
7bf260a
Compare
* Refine register methods, make Op can get rid of whole-archieve * `USE_OP` before a op is used. * Add unittest for add_op.
reyoung
force-pushed
the
feature/op_kernel_add
branch
from
7bf260a to
a0aaafe
Compare
jacquesqiao
reviewed
Jul 13, 2017
paddle/CMakeLists.txt
Outdated
| add_subdirectory(memory) | ||
| add_subdirectory(platform) | ||
| add_subdirectory(framework) | ||
| add_subdirectory(op) # because `operator` is a reserved word for CPP, so short to `op` |
There was a problem hiding this comment.
a personal suggestion, use ops instead of op
There was a problem hiding this comment.
Maybe operators is better.
jacquesqiao
reviewed
Jul 13, 2017
| ProtoMakerType(&op_proto, &op_checker); | ||
| PADDLE_ENFORCE(op_proto.IsInitialized(), | ||
| "Fail to initialize %s's OpProto !", op_type); | ||
| *op_proto.mutable_type() = op_type; |
There was a problem hiding this comment.
I am curious why this can work before without set op_type
gangliao
approved these changes
Jul 13, 2017
* Convert `op` --> `operators` * Remove AddType in OpProtoMaker, because type is part of registry. * Rename CPU_OR_GPU --> DEVICE_TYPE in registry macro.
QiJune
reviewed
Jul 14, 2017
| "REGISTER_OP must be in global namespace"); \ | ||
| static ::paddle::framework::OpRegisterHelper<__op_class, __op_maker_class> \ | ||
| __op_register_##__op_type##__(#__op_type); \ | ||
| int __op_register_##__op_type##_handle__() { return 0; } |
There was a problem hiding this comment.
这里加上这个函数是什么作用啊？
int __op_register_##__op_type##_handle__() { return 0; }
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
USE_OPbefore a op is used.