-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Function Adds some properties #1216
Conversation
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. Approve it to move fast.
|
||
check(inputs, outputs); | ||
// ArgType check still on here, | ||
// not sure whether it is better to put inside the check. |
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.
I think we can move the CHECK of argType into check funciton.
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.
我觉得先放calc里面吧,API的参数可能跟ASSIGN_TO/ADD_TO有关,放这里会清楚一些。
// And some Functions have the same input and output shapes, | ||
// so you may not need to enter the complete number of arguments. | ||
// But entering the full arguments is always correct for this interface. | ||
virtual size_t ops(const BufferArgs& inputs, const BufferArgs& outputs) { |
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.
Can we use timestamp to measure the running time of the 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.
嗯,后续再考虑如何获取运行实现,这里先描述operators数。
* Polish executor doc
Add FunctionBase::ops() and FunctionBase::check()
实现ISSUE中的特性
#892 (comment)