-
Notifications
You must be signed in to change notification settings - Fork 53
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 7th No.37】为 Paddle 代码转换工具新增 API 转换规则(第 4 组) #481
Conversation
请问如何查看CI的具体结果?我点进去是一片白的 @luotao1 |
|
@monster1015 日志已打开 |
|
paconvert/api_mapping.json
Outdated
"out" | ||
], | ||
"unsupport_args": [ | ||
"check_errors" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
文档改了这里不改?
paconvert/api_mapping.json
Outdated
"out" | ||
], | ||
"unsupport_args": [ | ||
"check_errors" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
文档改了这里不改?
paconvert/api_mapping.json
Outdated
"unsupport_args": [ | ||
"noncontiguous", | ||
"exclude_zero", | ||
"memory_format" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这些其实没必要全部算作 无法支持,只有影响到网络计算的才算。
paconvert/api_matcher.py
Outdated
return code | ||
|
||
|
||
class TensorMatrixExpMatcher(BaseMatcher): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个matcher能否复用class TensorFunc2PaddleFunc
paconvert/api_matcher.py
Outdated
else: | ||
kwargs["x"] = kwargs["input"] | ||
if "left" not in kwargs.keys(): | ||
kwargs["left"] = True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这些参数默认不是True吗,还是说默认参数不同,必须要设置一下
建议转写代码尽可能简洁无冗余
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
参数是一致的,我已经修改了,docs和对应的mathcer也已经修改
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
丰富下测试case,以下四种情况的测试case必须全部包含:
- 传入所有参数且全部指定关键字
- 传入所有参数且全部不指定关键字
- 改变关键字顺序
- 默认参数均不指定
paconvert/api_mapping.json
Outdated
"min_input_args": 1, | ||
"args_list": [ | ||
"A", | ||
"check_errors", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个不对吧,放在*的前后意义是不同的,是 位置参数 还是 指定关键字参数?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
他是指定关键字参数
paconvert/api_mapping.json
Outdated
"torch.Tensor.orgqr": {}, | ||
"torch.Tensor.ormqr": {}, | ||
"torch.Tensor.orgqr": { | ||
"Matcher": "OrgqrMatcher", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
命名TensorOrgqrMatcher
吧
[0.74611458, 1.24800785, 0.88039371]]) | ||
out1 = torch.randn(3, 3) | ||
info1 = torch.tensor([1, 2, 3], dtype=torch.int32) | ||
torch.linalg.cholesky_ex(a, check_errors=False, upper=True, out=(out1, info1)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
check_errors=True测一下
[0.48016702, 0.14235102, 0.42620817]]) | ||
out1 = torch.tensor([]) | ||
info1 = torch.tensor([1, 2, 3], dtype=torch.int32) | ||
out1, info1 = torch.linalg.inv_ex(x, check_errors=False, out=(out1, info1)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
check_errors=True的情况也测下
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
已经完善测试case了
已经全都跑通了 @zhwesky2010 |
paconvert/api_matcher.py
Outdated
API_TEMPLATE = textwrap.dedent( | ||
""" | ||
out = paddle.uniform({}, dtype={}, min={}, max={}).to({}) | ||
out.stop_gradient = not {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
如果没有requires_grad,这个地方可以简写为一行
有requires_grad才需要扩充为三行
参考下GenericMatcher
class LinalgCholeskyExMatcher(BaseMatcher): | ||
def generate_code(self, kwargs): | ||
if "upper" not in kwargs: | ||
kwargs["upper"] = False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个一定要单独设置吗?默认值不就是False吗,直接self.kwargs_to_str不就行了吗
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
paconvert/api_mapping.json
Outdated
] | ||
}, | ||
"torch.Tensor.ormqr": { | ||
"Matcher": "OrmqrMatcher", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个配置TensorFunc2PaddleFunc就可以吧
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个可以
paconvert/api_mapping.json
Outdated
@@ -13933,6 +14051,18 @@ | |||
"verbose" | |||
] | |||
}, | |||
"torch.ormqr": { | |||
"Matcher": "OrmqrMatcher", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个配置GenericMatcher就可以吧
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个也可以
paconvert/api_matcher.py
Outdated
return code | ||
|
||
|
||
class OrmqrMatcher(BaseMatcher): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个逻辑简单 应该不用单独写Matcher了,分别配置 TensorFunc2PaddleFunc和GenericMatcher 就行
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
paconvert/api_matcher.py
Outdated
@@ -807,6 +807,79 @@ def generate_code(self, kwargs): | |||
return code | |||
|
|||
|
|||
class TensorOrgqrMatcher(BaseMatcher): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个也可以用GenericMatcher,配置一个kwargs_change
paconvert/api_mapping.json
Outdated
"Matcher": "MakeTMatcher", | ||
"min_input_args": 3, | ||
"args_list": [ | ||
"shape", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个看起来shape是 可变参数 或 位置参数
其他的应该是 指定关键字参数
所以应该这么写:
"args_list": [
"*shape",
"*",
"device",
"dtype",
"low",
"high",
"requires_grad",
"noncontiguous",
"exclude_zero",
"memory_format"
]
只看torch文档可能不精确
""" | ||
) | ||
code = API_TEMPLATE.format( | ||
kwargs["shape"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这里面的这个shape比较复杂,因为其既可以是 可变参数:
torch.testing.make_tensor(3, 2, dtype=torch.float32, device=torch.device('cpu'))
也可以是 位置参数:
torch.testing.make_tensor([1, 2], dtype=torch.float32, device=torch.device('cpu'))
参考下CreateMatcher的写法吧,如果比较复杂不会写,可以看看是不是复用CreateMatcher,再配置下json字段就行
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
好的好的,我试试
已经好了,请review @zhwesky2010 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
PR Docs
PaddlePaddle/docs#6884
PR APIs