Skip to content
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

[XPU][PHI Kernels] bind reduce_max_int64 set_value_bool sin_grad_fp32 cos_grad_fp32 for XPU #55375

Merged
merged 5 commits into from
Jul 20, 2023

Conversation

lj970926
Copy link
Contributor

@lj970926 lj970926 commented Jul 12, 2023

PR types

New features

PR changes

OPs

Description

  1. 为XPU添加reduce_max/int64、set_value/bool、sin_grad/float32、cos_grad/float32
  2. set_value支持bool类型的过程中部分重构了xpu set_value_kernel实现并修改单测
    • 用xpu::broadcast替换xpu::broadcast_add(不支持bool类型),并删掉了不必要的api调用(xpu::stride_slice、xpu::constant)
    • 原来test_set_value_op_xpu里的XPUTestSetValueItemBool1~6对bool类型跑不过,GPU单测中对这几个单测只测试了fp32数据类型且在bool类型上也报与XPU相同的错误。因此将XPU单测对齐GPU、强制制定fp32类型。

@paddle-bot
Copy link

paddle-bot bot commented Jul 12, 2023

你的PR提交成功,感谢你对开源项目的贡献!
请关注后续CI自动化测试结果,详情请参考Paddle-CI手册
Your PR has been submitted. Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

@paddle-bot paddle-bot bot added contributor External developers status: proposed labels Jul 12, 2023
@paddle-bot
Copy link

paddle-bot bot commented Jul 12, 2023

❌ The PR is not created using PR's template. You can refer to this Demo.
Please use PR's template, it helps save our maintainers' time so that more developers get helped.

@@ -171,6 +173,8 @@ class XPUTestSetValueItemSliceInWhile(XPUTestSetValueApi):
def set_dtype(self):
if self.in_type == np.float16:
self.dtype = "float32"
elif self.in_type == np.bool_:
self.dtyper = "bool"
Copy link
Contributor

Choose a reason for hiding this comment

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

typo

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

@@ -57,6 +57,8 @@ def set_value(self):

def set_dtype(self):
self.dtype = self.in_type
if self.in_type == np.bool_:
Copy link
Contributor

Choose a reason for hiding this comment

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

没看明白是怎么只跑fp32的?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

这里对XPUTestSetValueItemBool重载了set_dtype,这几个测试跑只跑fp32,其他的单测还是测所有类型

@@ -166,7 +169,7 @@ void SetValueImpl(const Context& dev_ctx,
}
}

// Because strided_slice does not support the case of stride < 0
// Because strided_slice_view_update does not support the case of stride < 0
Copy link
Contributor

Choose a reason for hiding this comment

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

这块注释加到下面filp那里吧

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

value_dims_vec.end(),
ext_value_dims_vec.begin() + (max_dims - value_dims.size()));

r = xpu::broadcast<XPUType>(dev_ctx.x_context(),
Copy link
Contributor

Choose a reason for hiding this comment

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

这里的broadcast以及上面slice_dims_for_assign的处理最好封装在XPUElementwise函数里面,然后以functor的形式传进去,增加代码的复用程度

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

self.dtype = "float32"
else:
self.dtype = self.in_type
self.dtype = "float32"
Copy link
Contributor

Choose a reason for hiding this comment

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

这里为什么强制让类型是float32?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

如前面PR的description所述,添加了Bool类型后XPU单测挂了,后面我试过CPU也会报同样的错误且那边的单测只测float32
CPU那边的单测,TestItemBool继承了这里的set_dtype。
image
CPU的报错信息:
24594545f37b8785e38a1ca0f1f0e173
核心yr原因还是目前框架会在图里红圈的地方调用elementwise_sub,但是这个操作CPU上也不支持bool类型。换成FP32后结果就正常了。
image

Copy link
Contributor

@RuohengMa RuohengMa left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@ZibinGuo ZibinGuo left a comment

Choose a reason for hiding this comment

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

LGTM

@QingshuChen QingshuChen merged commit ab00c96 into PaddlePaddle:develop Jul 20, 2023
27 checks passed
@paddle-bot
Copy link

paddle-bot bot commented Jul 20, 2023

你的PR已合入Paddle库,请关注后续测试结果。
Your PR has been merged into the repository. An official integration test will be conducted later. Stay tuned.

cqulilujia pushed a commit to cqulilujia/Paddle that referenced this pull request Jul 24, 2023
… cos_grad_fp32 for XPU (PaddlePaddle#55375)

* bind kernels for xpu

* format code

* format code

* 0d support for set value

* refine set_value
wz1qqx pushed a commit to wz1qqx/Paddle that referenced this pull request Jul 31, 2023
… cos_grad_fp32 for XPU (PaddlePaddle#55375)

* bind kernels for xpu

* format code

* format code

* 0d support for set value

* refine set_value
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor External developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants