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.

Sign up for GitHub

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

Jump to bottom

【PaddlePaddle Hackathon 4】No.63 : add embedding fp16 test #51321

Merged
merged 18 commits into from
Aug 4, 2023
Merged

【PaddlePaddle Hackathon 4】No.63 : add embedding fp16 test #51321

merged 18 commits into from
Aug 4, 2023

Conversation

longranger2
Copy link
Contributor

@longranger2 longranger2 commented Mar 7, 2023 •
edited

PR types

Others

PR changes

APIs

Description

  • add embedding fp16 test

相关链接：
#51281
#54871

@paddle-bot
Copy link

paddle-bot bot commented Mar 7, 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 Mar 7, 2023
@paddle-bot
Copy link

paddle-bot bot commented Mar 7, 2023 •
edited

✅ This PR's description meets the template requirements!
Please wait for other CI results.

@longranger2 longranger2 mentioned this pull request Mar 7, 2023
Closed
@longranger2
Copy link
Contributor Author

longranger2 commented Mar 10, 2023 •
edited

@ZzSean @zhangting2020 想问下这个报错怎么解决呢？

image

@longranger2 longranger2 changed the title 【PaddlePaddle Hackathon 4】add embedding fp16 test and add lerp bf16 support 【PaddlePaddle Hackathon 4】No.63 add embedding fp16 test and add lerp bf16 support Mar 13, 2023
@longranger2 longranger2 changed the title 【PaddlePaddle Hackathon 4】No.63 add embedding fp16 test and add lerp bf16 support 【PaddlePaddle Hackathon 4】No.63 : add embedding fp16 test and add lerp bf16 support Mar 14, 2023
@longranger2 longranger2 changed the title 【PaddlePaddle Hackathon 4】No.63 : add embedding fp16 test and add lerp bf16 support 【PaddlePaddle Hackathon 4】No.64 : add embedding fp16 test and add lerp bf16 support Mar 15, 2023
@longranger2 longranger2 changed the title 【PaddlePaddle Hackathon 4】No.64 : add embedding fp16 test and add lerp bf16 support 【PaddlePaddle Hackathon 4】No.63 : add embedding fp16 test and add lerp bf16 support Mar 15, 2023
ZzSean
ZzSean reviewed Mar 15, 2023
View reviewed changes
paddle/phi/kernels/embedding_kernel.h Outdated
@@ -15,6 +15,7 @@
#pragma once

#include "paddle/phi/core/dense_tensor.h"
#include "paddle/phi/core/device_context.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

确认是否有必要加，下同

python/paddle/fluid/tests/unittests/test_lerp_op.py Outdated

def test_check_output(self):
place = core.CUDAPlace(0)
self.check_output_with_place(place, check_eager=True, atol=1e-2)
Copy link
Contributor

Choose a reason for hiding this comment

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

atol默认值无需设置

python/paddle/fluid/tests/unittests/test_lerp_op.py Outdated
['X', 'Y'],
'Out',
check_eager=True,
max_relative_error=1e-2,
Copy link
Contributor

Choose a reason for hiding this comment

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

同上

python/paddle/fluid/tests/unittests/test_lookup_table_v2_op.py Outdated
@@ -294,6 +294,40 @@ def test_param_dtype():
)


@unittest.skipIf(
Copy link
Contributor

Choose a reason for hiding this comment

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

fp16不用加跳过

python/paddle/fluid/tests/unittests/test_lookup_table_v2_op.py Outdated
or not core.is_float16_supported(core.CUDAPlace(0)),
"core is not complied with CUDA and not support the float16",
)
class TestEmbeddingFP16OP(OpTest):
Copy link
Contributor

Choose a reason for hiding this comment

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

同之前建议，直接继承TestLookupTableOp

@longranger2
Copy link
Contributor Author

longranger2 commented Mar 21, 2023 •
edited

@ZzSean 想问下CI-Coverage的应该如何解决？

image

@longranger2 longranger2 requested review from ZzSean and removed request for ZzSean March 22, 2023 04:09
@longranger2
Copy link
Contributor Author

@ZzSean 辛苦review下～

ZzSean
ZzSean reviewed Apr 18, 2023
View reviewed changes
python/paddle/fluid/tests/unittests/test_lerp_op.py Outdated
or not core.is_bfloat16_supported(core.CUDAPlace(0)),
"core is not complied with CUDA and not support the bfloat16",
)
class TestLerpBF16(OpTest):
Copy link
Contributor

Choose a reason for hiding this comment

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

case太少，需要跟FP16对齐数量

ZzSean
ZzSean reviewed Apr 18, 2023
View reviewed changes
python/paddle/fluid/tests/unittests/test_lookup_table_v2_op.py Outdated
table = np.random.random((17, 31)).astype("float64")
self.init_dtype()

table = np.random.random((17, 32)).astype(self.dtype)
Copy link
Contributor

Choose a reason for hiding this comment

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

不能因为原始测例无法通过，就改测例啊

@ZzSean
Copy link
Contributor

ZzSean commented Apr 18, 2023

broadcast_tensor没添加单测

@longranger2
Copy link
Contributor Author

broadcast_tensor没添加单测

该PR是的目的是： add embedding fp16 test and add lerp bf16 support
至于为broadcast_tensor添加bfloat16是因为lerp间接用到，但其不支持bf16，因此进行添加

image

@longranger2 longranger2 changed the title 【PaddlePaddle Hackathon 4】No.63 : add embedding fp16 test and add lerp bf16 support 【PaddlePaddle Hackathon 4】No.63 : add embedding fp16 test Apr 19, 2023
@paddle-ci-bot
Copy link

paddle-ci-bot bot commented Apr 29, 2023

Sorry to inform you that f4cdd47's CIs have passed for more than 7 days. To prevent PR conflicts, you need to re-run all CIs manually.

@zhangting2020 zhangting2020 mentioned this pull request Jun 26, 2023
Closed
@longranger2
Copy link
Contributor Author

@zhangting2020 辛苦帮忙review下～，PR-CI-Coverage的报错不知道怎么解决

image

zhangting2020
zhangting2020 reviewed Aug 1, 2023
View reviewed changes
test/legacy_test/test_lookup_table_v2_op.py
table = np.random.random((17, 31)).astype("float64")
self.init_dtype()

table = np.random.random((17, 31)).astype(self.dtype)
Copy link
Contributor

Choose a reason for hiding this comment

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

这个算子低精度实现存在Bug，对于奇数元素数量的场景处理存在问题。先在低精度的case里面，换一个偶数元素的shape吧，我们后续修复这个算子问题。

另外还有bf16的单测

@niuliling123 niuliling123 mentioned this pull request Aug 3, 2023
Merged
@longranger2
Copy link
Contributor Author

@zhangting2020 修改好了，辛苦review下～

@luotao1 luotao1 merged commit 9f2d88e into PaddlePaddle:develop Aug 4, 2023
27 checks passed
cxxly pushed a commit to cxxly/Paddle that referenced this pull request Aug 7, 2023
@longranger2 @cxxly
【PaddlePaddle Hackathon 4】No.63 : add embedding fp16 test (PaddlePadd…
c569f07 
…le#51321)
cxxly pushed a commit to cxxly/Paddle that referenced this pull request Aug 7, 2023
@longranger2 @cxxly
【PaddlePaddle Hackathon 4】No.63 : add embedding fp16 test (PaddlePadd…
4e00303 
…le#51321)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ZzSean ZzSean ZzSean left review comments

@zhangting2020 zhangting2020 zhangting2020 approved these changes

Assignees

@luotao1 luotao1

@Vvsmile Vvsmile

@zhangting2020 zhangting2020

@ZzSean ZzSean

@Ligoml Ligoml

Labels
contributor External developers PaddlePaddle Hackathon
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@longranger2 @ZzSean @zhangting2020 @luotao1 @Vvsmile @Ligoml