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

【Hackathon No.46】为 Paddle gumbel_softmax 算子实现 float16 数据类型支持 #50923

Merged
merged 5 commits into from
Mar 17, 2023

Conversation

denglianbin
Copy link
Contributor

@denglianbin denglianbin commented Feb 26, 2023

PR types

Others

PR changes

Others

Describe

性能数据(op benchmark)

shape hard axis fp32 fp16
128, 128 TRUE -1 0.088926481 0.070158559
100 TRUE -1 0.059882962 0.052535534
20, 10, 5 TRUE 1 0.064530178 0.057325801
20, 10, 5 FALSE 1 0.051758971 0.047544071

@paddle-bot
Copy link

paddle-bot bot commented Feb 26, 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.

@CLAassistant
Copy link

CLAassistant commented Feb 26, 2023

CLA assistant check
All committers have signed the CLA.

@paddle-bot paddle-bot bot added contributor External developers status: proposed labels Feb 26, 2023
@denglianbin denglianbin changed the title 【Hackathon + No.任务编号】为 Paddle gumbel_softmax 算子实现 float16 数据类型支持 【Hackathon No.46】为 Paddle gumbel_softmax 算子实现 float16 数据类型支持 Feb 26, 2023
@zhangting2020 zhangting2020 self-requested a review March 6, 2023 05:24
index_sequence_begin + size,
thrust::device_ptr<T>(random_data),
UniformCUDAGenerator<T>(static_cast<phi::dtype::float16>(0.00001),
static_cast<phi::dtype::float16>(1),
Copy link
Contributor

Choose a reason for hiding this comment

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

这里是否有修改的必要?无论T为何种类型,这里都cast到FP16

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.count_expected = 100
self.dtype = np.float16


Copy link
Contributor

Choose a reason for hiding this comment

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

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

@zhangting2020
Copy link
Contributor

另外记得提交中文文档的修改PR,可以在这个PR说明中引用一下或者提交PR后assign给我,我会review

@denglianbin
Copy link
Contributor Author

@zhangting2020 老师您好,这是中文文档更新pr: PaddlePaddle/docs#5707

self.check_output()

def test_check_grad(self):
self.check_grad(["X"], "Out")
Copy link
Contributor

Choose a reason for hiding this comment

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

FP16的单测需要继承TestGumbelSoftmaxOp,实际上只需要为fp16的case重写init_attrs,可以减少冗余代码。
TestGumbelSoftmax_ZeroDim_FP16OP -> TestGumbelSoftmaxFP16OP

Copy link
Contributor Author

Choose a reason for hiding this comment

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

老师,您好,这里是参考单测中原来写法。针对于ZeroDim单独继承optest进行测试,其余各test继承TestGumbelSoftmaxOp并重写init_attr()。我这里也是针对于ZeroDim单独处理了。所以直接继承了optest。后续四个test都是直接继承TestGumbelSoftmaxOp并重写init_attr()的。

Copy link
Contributor

Choose a reason for hiding this comment

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

原始写法我想并不是最优的。TestGumbelSoftmax_ZeroDim里面其实重写init_attr也可以吧

Copy link
Contributor Author

Choose a reason for hiding this comment

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

老师您好,我尝试了直接用TestGumbelSoftmax_ZeroDim继承TestGumbelSoftmaxOp基类,但是由于基类中check_out是针对多维重写的check_out_custormized,并不适用于ZeroDim。因此我在TestGumbelSoftmax_ZeroDim中添加了init_attr方法,并令TestGumbelSoftmax_ZeroDimFP16继承修改。

self.attrs = {"hard": True, "axis": 1}
self.count_expected = 100
self.dtype = np.float16

Copy link
Contributor

Choose a reason for hiding this comment

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

这4个单测继承TestGumbelSoftmaxFP16OP。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

您好,因为前面TestGumbelSoftmax_ZeroDim_FP16OP是针对于ZeroDim的,所以内部没有init_attrs()函数。无法更改名字为TestGumbelSoftmaxFP16OP。所以直接继承自TestGumbelSoftmaxOp。

@denglianbin
Copy link
Contributor Author

image
@zhangting2020 您好,请问static_check这条CI中会报这个错误,看着是CI环境镜像问题。本地尝试了报错的sample,没有问题。这种问题需要做什么处理吗?

@luotao1
Copy link
Contributor

luotao1 commented Mar 16, 2023

请问static_check这条CI中会报这个错误,看着是CI环境镜像问题

已解决,参考 #51512 (comment) 。目前流水线已经是正常的了。 @denglianbin

@denglianbin
Copy link
Contributor Author

请问static_check这条CI中会报这个错误,看着是CI环境镜像问题

已解决,参考 #51512 (comment) 。目前流水线已经是正常的了。 @denglianbin

好的老师,已经重新跑了,最后报错是find RD for approval first了,应该是正常的了。

Copy link
Contributor

@zhangting2020 zhangting2020 left a comment

Choose a reason for hiding this comment

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

LGTM

@luotao1 luotao1 merged commit e0007f3 into PaddlePaddle:develop Mar 17, 2023
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.

6 participants