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

[NewIR] add_n and combine support selected rows #56754

Merged
merged 13 commits into from
Sep 6, 2023

Conversation

wanghuancoder
Copy link
Contributor

PR types

New features

PR changes

Others

Description

调通test_simnet和test_simnet_v2
add_n和combine支持了SelectedRows
Pcard-67164

@paddle-bot
Copy link

paddle-bot bot commented Aug 29, 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.

@@ -195,3 +195,5 @@ test_where_op
test_yolo_box_op
test_yolov3_loss_op
test_fill_constant_op
test_simnet
Copy link
Contributor

Choose a reason for hiding this comment

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

动转静单测的话修改这里没有用,可以参考 #56631 。目前新IR对动转静单测的监控是通过添加装饰器进行的

@Aurelius84 Aurelius84 changed the title [IR] add_n and combine support selected rows [NewIR] add_n and combine support selected rows Sep 6, 2023
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.

LGTM


VLOG(4) << "Builder construction outputs";
ir::VectorType inputs = inputs_.type().dyn_cast<ir::VectorType>();
(void)inputs;
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.

好的我先合入,我再删掉这行


VLOG(4) << "Builder construction outputs";
ir::VectorType inputs = inputs_.type().dyn_cast<ir::VectorType>();
(void)inputs;
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

@kangguangli kangguangli left a comment

Choose a reason for hiding this comment

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

LGTM overall

@@ -1112,8 +1112,8 @@ struct AddNOpTranscriber : public OpTranscriber {
}
const auto& op_info = ctx->GetRegisteredOpInfo(target_op_name);
if (!op_info) {
IR_THROW(
"Op assign_value should have corresponding OpInfo pd.assign_value_");
IR_THROW("Op assign_value should have corresponding OpInfo %s",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
IR_THROW("Op assign_value should have corresponding OpInfo %s",
IR_THROW("Op %s should have corresponding OpInfo %s",

@wanghuancoder wanghuancoder merged commit 85dbcef into PaddlePaddle:develop Sep 6, 2023
BeingGod pushed a commit to BeingGod/Paddle that referenced this pull request Sep 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants