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】modify static_op_function vector mutable attr bug (Split op ) #57218

Closed

Conversation

xiaoguoguo626807
Copy link
Contributor

@xiaoguoguo626807 xiaoguoguo626807 commented Sep 12, 2023

PR types

Bug fixes

PR changes

others

Description

pcard-67164

  1. 可变attribute相关修复
    修改python-c 接口代码,将可变attribute 在python-c层全部转为opreslut 传递给pd_api 接口。由于一个op可能有多个可变attribute,无法限制用户全部使用同一形式传参,而支持各个组合会使代码分支增多,因此统一处理为输入形式。
    修改前生成代码为
image 修改后生成代码为 截屏2023-09-01 14 09 44 由于需要将用户传入的attr 形式处理为opreslut,在此处调用fill_int_array接口和full 接口,其中fill_int_array 返回的为vectortype的opresult; 而用户直接传入input 形式的为vector 为了保持相同变量的类型相同,补充combine op 实现vector 到 opresult 的转换。

同时,若vector 类型的attrbute 以input输入,pd_api.h 提供的接口应该为 std::vector<ir::OpResult >形式; 但static_op_function调用形式为 OpResult, 因此生成两种可变attribute input 形式的接口,加attr 形式接口共三个。
截屏2023-09-01 14 15 13

2.split python api 适配
3.ir_backward 修复full 无法创建空shape,-1shape opreslut 问题

此pr 拆解为 #57281 #57333 comments 回复#57354

xiaoguoguo626807 and others added 30 commits August 11, 2023 07:40
@paddle-bot
Copy link

paddle-bot bot commented Sep 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.

changeyoung98
changeyoung98 previously approved these changes Sep 14, 2023
Copy link
Contributor

@changeyoung98 changeyoung98 left a comment

Choose a reason for hiding this comment

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

LGTM

YuanRisheng
YuanRisheng previously approved these changes Sep 14, 2023
@@ -132,19 +132,29 @@ def _gen_api_inputs(self, op_info):
ret.append(f'{self._type_map[type]} {name}')
return ', '.join(ret)

def _gen_api_attrs(self, op_info, with_default, is_mutable_attr):
def _gen_api_attrs(
self, op_info, with_default, is_mutable_attr, is_vector_mutable_sttr
Copy link
Contributor

Choose a reason for hiding this comment

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

is_vector_mutable_sttr -> is_vector_mutable_attr

mutable_attr.append(f'{OP_RESULT} {name}')
if (
mutable_type_list[mutable_name_list.index(name)][0]
== "paddle::dialect::IntArrayAttribute"
Copy link
Contributor

Choose a reason for hiding this comment

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

发现该pr有大量这样的写法,可以把"paddle::dialect::IntArrayAttribute"抽出来变成全局变量,或者是封装成函数,避免硬编码

@YuanRisheng
Copy link
Contributor

YuanRisheng commented Sep 14, 2023


为何需要这里的第三种带有vector OpResult 的接口?

@@ -132,19 +132,29 @@ def _gen_api_inputs(self, op_info):
ret.append(f'{self._type_map[type]} {name}')
return ', '.join(ret)

def _gen_api_attrs(self, op_info, with_default, is_mutable_attr):
def _gen_api_attrs(
self, op_info, with_default, is_mutable_attr, is_vector_mutable_sttr
Copy link
Contributor

Choose a reason for hiding this comment

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

is_vector_mutable_attr?

@@ -138,6 +138,25 @@ bool PyObject_CheckIROpResult(PyObject* obj) {
return PyObject_TypeCheck(obj, g_ir_opresult_pytype);
}

bool PyObject_CheckIRVectorOfOpResult(PyObject* obj) {
if (PyList_Check(obj)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

这里需要补充对于tuple的处理吗?

}
for (Py_ssize_t i = 0; i < len; i++) {
item = PyList_GetItem(obj, i);
if (!PyObject_CheckIROpResult(item)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

现在可以处理python端传过来的[1, OpResult, 2]这样int和OpResult混合的list吗?

This was referenced Sep 15, 2023
@xiaoguoguo626807 xiaoguoguo626807 deleted the split_first branch September 22, 2023 01:36
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.

5 participants