Skip to content

[PIR]Gen check DataType #59354

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
Merged
merged 19 commits into from
Dec 4, 2023
Merged

[PIR]Gen check DataType #59354

merged 19 commits into from
Dec 4, 2023

Conversation

zrr1999
Copy link
Member

@zrr1999 zrr1999 commented Nov 24, 2023 •
edited
Loading

PR types

Others

PR changes

Others

Description

[PIR]Gen check DataType

根据yaml中的配置情况，有下面4中情况需要支持check dtype（已经全部完成）：

  • yaml中kernel下指定了data_type，且指定的data_type是某个input，比如numel （之前的PR中支持了这种情况）
pir::OpResult abs(const pir::Value& x){
    CheckValueDataType(x, "x", "abs");
    paddle::dialect::AbsOp abs_op = ApiBuilder::Instance().GetBuilder()->Build<paddle::dialect::AbsOp>(x);
    return abs_op.result(0);
}
  • yaml中kernel下指定了data_type，但是指定的data_type不是input，而是dtype attr，比如empty，需要对dtype attr进行check（本PR中支持了这种情况）
pir::OpResult empty(std::vector<pir::Value> shape, phi::DataType dtype, const Place& place){
    CheckDataType(dtype, "dtype", "empty");
    auto shape_combine_op = ApiBuilder::Instance().GetBuilder()->Build<pir::CombineOp>(shape);
    paddle::dialect::EmptyOp empty_op = ApiBuilder::Instance().GetBuilder()->Build<paddle::dialect::EmptyOp>(shape_combine_op.out(), dtype, place);
    return empty_op.result(0);
}
  • 指定了data_type但有先后顺序，比如empty_like，可能需要生成ifelse，按顺序对input和attr进行check（本PR中支持了这种情况）
pir::OpResult empty_like(const pir::Value& x, phi::DataType dtype, const Place& place){
    CheckDataTypeOrValue(dtype, "dtype", x, "x", "empty_like");
    paddle::dialect::EmptyLikeOp empty_like_op = ApiBuilder::Instance().GetBuilder()->Build<paddle::dialect::EmptyLikeOp>(x, dtype, place);
    return empty_like_op.result(0);
}
  • yaml中没有指定data_type，比如logsumexp，计划先只check最后一个input进行check，因为选kernel会根据最后一个input的dtype选择（本PR中支持了这种情况）

这里input是指的Value类型的输入参数，其他类型的参数被称为attr

参考 #58954

@paddle-bot Paddle-bot
Copy link

paddle-bot bot commented Nov 24, 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 the contributor External developers label Nov 24, 2023
@zrr1999 zrr1999 requested a review from 0x45f November 27, 2023 05:26
@0x45f
Copy link
Contributor

0x45f commented Nov 27, 2023 •
edited
Loading

Great job！！！
不过覆盖率没有过，建议针对支持的第二、三类的api添加error类的pir单测。可以参考 #59172 。对于 #59172 中提到的新增C++端错误类型的事情，可以等我们这4类check情况都支持完毕后再添加错误类型~
顺便在pr描述中贴一下对于二、三类情况生成的api代码是怎么样的吧~

0x45f
0x45f reviewed Nov 27, 2023
View reviewed changes
@zrr1999
Copy link
Member Author

zrr1999 commented Nov 27, 2023

#59172

好的，我去加一下

@0x45f 0x45f self-assigned this Nov 28, 2023
@0x45f
Copy link
Contributor

0x45f commented Nov 28, 2023

对于第四种情况，从CI看编译还有些问题，以ci中编译报错的bilinear这个API为例。pd_api.cc中这个api的签名如下

pir::OpResult bilinear(const pir::Value& x, const pir::Value& y, const pir::Value& weight, const paddle::optional<pir::Value>& bias)

最后一个输入类型是paddle::optionalpir::Value的，paddle::optional的意思是说bias这个参数可能是null。对于bilinear会根据最后一个输入的dtype选择kernel，如果bias为null那就会根据weight的dtype来选择kernel。那么对于bilinear这个api来说，生成的检查逻辑可能类似下面：

if (!bias) {
    check(bias)
} else {
    check(weight)
}

另外：我不确定是不是存在这样的API，比如bilinear的weight参数也是optional的，那么生成的逻辑是不是需要多个ifelse呢？可以先看下是否会有这样的API

@0x45f
Copy link
Contributor

0x45f commented Nov 28, 2023

对于第四种情况，从CI看编译还有些问题，以ci中编译报错的bilinear这个API为例。pd_api.cc中这个api的签名如下

pir::OpResult bilinear(const pir::Value& x, const pir::Value& y, const pir::Value& weight, const paddle::optional<pir::Value>& bias)

最后一个输入类型是paddle::optionalpir::Value的，paddle::optional的意思是说bias这个参数可能是null。对于bilinear会根据最后一个输入的dtype选择kernel，如果bias为null那就会根据weight的dtype来选择kernel。那么对于bilinear这个api来说，生成的检查逻辑可能类似下面：

if (!bias) {
    check(bias)
} else {
    check(weight)
}

另外：我不确定是不是存在这样的API，比如bilinear的weight参数也是optional的，那么生成的逻辑是不是需要多个ifelse呢？可以先看下是否会有这样的API

或者如果optional相关的api比较少这里，支持起来又比较麻烦的话，可以先加个黑名单之类的，跳过这类api。等下一个pr在支持这类情况~

@zrr1999 zrr1999 closed this Nov 28, 2023
@zrr1999 zrr1999 reopened this Nov 28, 2023
0x45f
0x45f reviewed Nov 29, 2023
View reviewed changes
@0x45f
Copy link
Contributor

0x45f commented Nov 29, 2023

test_conj_op.py中Testfp16ConjOp.testfp16 这个单测修改一下，只在gpu环境下跑吧，可以用is_compiled_with_cuda这个api判断。因为这个cpu kernel没有注册fp16类型~

@zrr1999
Copy link
Member Author

zrr1999 commented Nov 29, 2023

test_conj_op.py中Testfp16ConjOp.testfp16 这个单测修改一下，只在gpu环境下跑吧，可以用is_compiled_with_cuda这个api判断。因为这个cpu kernel没有注册fp16类型~

好的，已修改

0x45f
0x45f reviewed Nov 30, 2023
View reviewed changes
0x45f
0x45f reviewed Dec 1, 2023
View reviewed changes
Aurelius84
Aurelius84 approved these changes Dec 4, 2023
View reviewed changes
Copy link
Contributor

@Aurelius84 Aurelius84 left a comment

Choose a reason for hiding this comment

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

Great work!

@0x45f 0x45f merged commit 05a3536 into PaddlePaddle:develop Dec 4, 2023
SigureMo pushed a commit to gouzil/Paddle that referenced this pull request Dec 5, 2023
@zrr1999 @SigureMo
[PIR]Gen check DataType (PaddlePaddle#59354)
1452248
@zrr1999 zrr1999 mentioned this pull request Dec 8, 2023
Closed
20 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@0x45f 0x45f 0x45f left review comments

@Aurelius84 Aurelius84 Aurelius84 approved these changes

@YuanRisheng YuanRisheng YuanRisheng approved these changes

@tianshuo78520a tianshuo78520a tianshuo78520a approved these changes

@zhangbo9674 zhangbo9674 zhangbo9674 approved these changes

Assignees

@0x45f 0x45f

Labels
contributor External developers
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@zrr1999 @0x45f @Aurelius84 @YuanRisheng @tianshuo78520a @zhangbo9674