-
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
add sin and cos optional parameters to fused_rope op #55415
Conversation
你的PR提交成功,感谢你对开源项目的贡献! |
test/legacy_test/_run.log
Outdated
@@ -0,0 +1,2 @@ | |||
timeout: invalid option -- '1' | |||
Try 'timeout --help' for more information. |
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.
这个文件删除掉
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.
收到 已删除
@@ -124,6 +124,68 @@ def get_forward_backward(self, rope_function, seed): | |||
|
|||
return fw, bw | |||
|
|||
def get_forward_backward_sin_cos(self, rope_function, seed): |
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.
和上面的重复代码有点多, 可以把共用的抽出来
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.
已修改
|
||
for (; index < size; index += stride) { | ||
#pragma unroll | ||
for (int nx = 0; nx < VecSize; ++nx) { |
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.
VectorizedFusedRopeWithSinCosKernel
和VectorizedFusedRopeKernel
只有L46-L56、L108-L117的实现不同,其他代码都一样,没有必要定义成两个函数,代码尽量复用一下。- sin、cos计算的逻辑,算子前、反向也是一样的,可以挪到头文件中以复用
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.
已修改
…& move VectorizedFusedRopeKernel to fused_rope_utils.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
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 for the changes of Inputs/Output/Attrs of OPs
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 for docs
- 补充一下中文文档吧
PR types
Others
PR changes
Others
Description
Pcard-70458
fused_rope算子功能加强,支持传入optional的sin、cos输入