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

【Hackathon 7th No.38】为 Paddle 代码转换工具新增 API 转换规则(第5组) #487

Closed
wants to merge 4 commits into from

Conversation

inaomIIsfarell
Copy link
Contributor

@inaomIIsfarell inaomIIsfarell commented Sep 26, 2024

PR Docs

PaddlePaddle/docs#6885

PR APIs

torch.isposinf
torch.isneginf
torch.isreal
torch.isin
torch.Tensor.isposinf
torch.Tensor.isneginf
torch.Tensor.isreal
torch.Tensor.scatter_reduce
torch.scatter_reduce
torch.positive
torch.Tensor.positive
torch.concatenate
torch.can_cast
torch.float_power
torch.block_diag
torch.cartesian_prod

ps: 如果有朋友用 vscode 修改 paconvert/api_mapping.jsonpaconvert/api_alias_mapping.json,并用 pre-commit commit 时,出现 json 文件diff 全覆盖的情况,可能 是 pre-commit 的问题

@inaomIIsfarell
Copy link
Contributor Author

inaomIIsfarell commented Sep 30, 2024

CI-codestyle 里显示没用pre commit导致报错,but我用pre commit 的话会导致json文件全覆盖(p1),例子p2中可以看到diff显示全覆盖,但实际上只有一行进行了更改
jsondiff_precommit
example

Copy link
Collaborator

@zhwesky2010 zhwesky2010 left a comment

Choose a reason for hiding this comment

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

  1. 写法上注意优化下,目前有些过于hard code
  2. 转换后的代码注意简洁下,尽可能一行对一行,不要加太多其他的内容及判断等等,这会导致转换后的代码比较丑
  3. 丰富下测试case,以下四种情况的测试case必须全部包含:
  • 传入所有参数且全部指定关键字
  • 传入所有参数且全部不指定关键字
  • 改变关键字顺序
  • 默认参数均不指定

@@ -1450,6 +1450,14 @@ def generate_code(self, kwargs):
return GenericMatcher.generate_code(self, kwargs)


class ScatterReduceMatcher(BaseMatcher):
def generate_code(self, kwargs):
reduce_mapping = {'"""sum"""': '"add"', '"""prod"""': '"multiply"'}
Copy link
Collaborator

Choose a reason for hiding this comment

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

这个可能得用辅助函数形式写,因为传入的reduce可能是个变量

例如:

reduce_type = 'sum'
torch.scatter_reduce(reduce=reduce_type, ...)

class CartesianProdMatcher(BaseMatcher):
def get_paddle_nodes(self, args, kwargs):
new_args = self.parse_args(args)
code = "paddle.cartesian_prod([ {}".format(new_args[0])
Copy link
Collaborator

Choose a reason for hiding this comment

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

  1. 如果输入的是变量形式,应该无法支持,参考 Chain_MatmulMatcher
  2. new_args返回的就是一个list,是不是直接format到字符串,目前这样写代码有点丑

return "{}.cast(paddle.float64).pow({})".format(
self.paddleClass, kwargs["exponent"]
self.write_aux_code()
_from_dtype = kwargs["from_"][3:-3]
Copy link
Collaborator

Choose a reason for hiding this comment

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

这么写代码太硬了,一定要这样写死切片吗,如果输入类型是变量,会出错吧,例如:

dtype1=torch.float32
dtype2=torch.float64
torch.can_cast(dtype1, dtype2)



class PositiveMatcher(BaseMatcher):
def generate_code(self, kwargs):
Copy link
Collaborator

Choose a reason for hiding this comment

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

这个这么多行,还是用辅助函数来写吧,不然很容易破坏原代码的结构

)
else:
if "out" not in kwargs:
return "paddle.pow({}.cast(paddle.float64), {}.cast(paddle.float64) if isinstance({}, paddle.Tensor) else {})".format(
Copy link
Collaborator

Choose a reason for hiding this comment

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

exponent也必须cast吗,只用cast input就行吧,这个应该自带类型提升

Copy link
Contributor Author

Choose a reason for hiding this comment

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

应该不行,paddle.pow(a, b) 中,ab 都是 tensor 的时候数据类型必须一致,否则会报错

@zhwesky2010
Copy link
Collaborator

@inaomIIsfarell CI未通过且出现冲突,需要修改

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor External developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants