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

【Paddle Tensor 第二期 常用API复数类型支持 NO.6】 添加 full 函数复数类型支持 #70277

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

MrXnneHang
Copy link
Contributor

PR Category

User Experience

PR Types

New features

Description

更简洁的实现。

Copy link

paddle-bot bot commented Dec 17, 2024

你的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 Dec 17, 2024
elif isinstance(fill_value, (builtins.complex)):
dtype = "complex128"
else:
dtype = paddle.get_default_dtype()
Copy link
Contributor Author

@MrXnneHang MrXnneHang Dec 17, 2024

Choose a reason for hiding this comment

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

按照规范化测试,full默认 dtype 应该是float64int64complex128
但是似乎这个会让很多旧有的引用到full测试失败,大多数是调用时没有指定dtype最终导致与要求输入类型不匹配引起的。
image
特别是原本似乎存在这样的Bug。不指定bool的话True会被默认转成float,这就有点离谱了。

我先把我自己的代码测一遍,之后再考虑default dtype的问题。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

如果需要修改默认dtype,特别是bool的,我可以再添加。
我应该可以修复遗留的test问题。应该只要给没有指定 dtype 的调用加上一个dtype="float32"就行。

Copy link
Contributor

@HydrogenSulfate HydrogenSulfate Dec 18, 2024

Choose a reason for hiding this comment

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

按照规范化测试,full默认 dtype 应该是float64int64complex128。 但是似乎这个会让很多旧有的引用到full测试失败,大多数是调用时没有指定dtype最终导致与要求输入类型不匹配引起的。 image 特别是原本似乎存在这样的Bug。不指定bool的话True会被默认转成float,这就有点离谱了。

我先把我自己的代码测一遍,之后再考虑default dtype的问题。

这个默认的dtype建议不要改动,否则会引起已有模型的兼容性问题,如果存在兼容性问题,我这边可以选择跳过这个单测。bool转float的错误情况除外

Copy link
Contributor Author

@MrXnneHang MrXnneHang Dec 18, 2024

Choose a reason for hiding this comment

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

那么默认 dtype 我就不动了。我去把bool加上去。

@MrXnneHang
Copy link
Contributor Author

@HydrogenSulfate
我把PR挪了地方

@luotao1 luotao1 added the HappyOpenSource 快乐开源活动issue与PR label Dec 18, 2024
Copy link
Contributor

@HydrogenSulfate HydrogenSulfate left a comment

Choose a reason for hiding this comment

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

  1. full不是已经支持了复数了吗?
  2. 这个PR解决了什么问题呢?

@MrXnneHang
Copy link
Contributor Author

MrXnneHang commented Dec 18, 2024

  1. full不是已经支持了复数了吗?
  2. 这个PR解决了什么问题呢?

1.full的动态图本身已经支持复数了。但是对于静态图,它不支持。静态图API是手动定义的,位于:

PyObject *static_api_full(PyObject *self, PyObject *args, PyObject *kwargs) {

由于它直接调用的paddle::dialect::full是根据ops.yaml生成的(位于build/paddle/fluid/pir/dialect/operator/ir/pd_api.cc),Scalar写死成double,所以静态图会报以下错误:
image

int,float等都可以隐式自动转换到double是没有问题的,但是complex->double的直接转换是不被允许的。

double value = CastPyArg2Double(value_obj, "full", 1);

这一行把复数拦住了。

double CastPyArg2Double(PyObject* obj,
                        const std::string& op_type,
                        ssize_t arg_pos) {
  if (PyObject_CheckFloat(obj)) {
    return PyObject_ToDouble(obj);  // NOLINT
  } else {
    PADDLE_THROW(common::errors::InvalidType(
        "%s(): argument (position %d) must be "
        "double, but got %s",
        op_type,
        arg_pos + 1,
        ((PyTypeObject*)obj->ob_type)->tp_name));  // NOLINT
  }

  return 0.0;
}

2.让静态图支持复数输入,顺手修了一个complex自动被转成float的bug,增加了paddle.full复数类型的单测。
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor External developers HappyOpenSource 快乐开源活动issue与PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants