-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
Adding increment op #4940
Adding increment op #4940
Conversation
paddle/operators/increment_op.cc
Outdated
The equation is: Out = X + step | ||
)DOC"); | ||
AddAttr<AttrType>("step", "The scaling factor of the scale operator.") | ||
.SetDefault(1.0); |
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.
The comments are not correct. The step is not the scaling factor. Maybe We can implement this math operation in the https://github.com/PaddlePaddle/Paddle/blob/develop/paddle/operators/activation_op.h .
And maybe the name activation_op
is not an accurate name, all these activations can be also used as the math operation.
Another option, we can modify the scale operator, let it support:
y = a * x + b
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.
Thanks for pointing out the mistake in the comment. I forgot to change it. I will fix it immediately.
@qingqing01 I really like your idea of changing the scale op. However, then we might have to rename it because the word scale
mean will only imply the multiplicative factor.
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.
Oh, you are right. One operator is benefit to save memory. But I approve this PR.
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.
Fixes #4912