PaddlePaddle / Paddle

PArallel Distributed Deep LEarning: Machine Learning Framework from Industrial Practice (『飞桨』核心框架,深度学习&机器学习高性能单机、分布式训练和跨平台部署)
http://www.paddlepaddle.org/
Apache License 2.0
21.66k stars 5.44k forks source link

【Hackathon No.6】implement nan_to_num #42469

Closed tiancaishaonvjituizi closed 1 year ago

tiancaishaonvjituizi commented 2 years ago

PR types

New features

PR changes

OPs

Describe

实现 nan_to_num

Hackathon issue:https://github.com/PaddlePaddle/Paddle/issues/40329 Hackathon RFC:https://github.com/PaddlePaddle/community/pull/93

paddle-bot-old[bot] commented 2 years ago

你的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.

zhwesky2010 commented 2 years ago

GPU下现在编不过,需要修一下 infoflow 2022-05-09 19-19-00

tiancaishaonvjituizi commented 2 years ago

GPU下现在编不过,需要修一下

这个错误我完全没有头绪,只在 windows 下会出现,涉及到的函数 ”_fdtest“ 并不是我写的,而且我的写法和 eye_kernel_impl.h 是一样的,不知道为什么会出错。paddle 官方开发者对这个错误有什么想法吗

zhwesky2010 commented 2 years ago

GPU下现在编不过,需要修一下

这个错误我完全没有头绪,只在 windows 下会出现,涉及到的函数 ”_fdtest“ 并不是我写的,而且我的写法和 eye_kernel_impl.h 是一样的,不知道为什么会出错。paddle 官方开发者对这个错误有什么想法吗

这个是说不能从__global__里调__host__主机函数,推测是std::isnanstd::numeric_limits<T>::infinity 这两个函数的问题

tiancaishaonvjituizi commented 2 years ago

@zhouwei25 我已经参考 pytorch 修改了 isnan 相关的实现,现在应该没有问题了。但现在 ci 有很多奇怪的错误(网络问题、test_parallel_dygraph_unused_variables_gloo 超时等),该怎么操作呢

tiancaishaonvjituizi commented 2 years ago

@zhouwei25 现在 CI 几乎都过了

tiancaishaonvjituizi commented 2 years ago

@zhouwei25 所有 CI 已过,只差 api 改动所需的 approve 了

Ligoml commented 2 years ago

@tiancaishaonvjituizi 需要在paddle/docs下同步添加中文文档哈,这里有一些参考文档:API书写规范文档预览工具中文API文档复制英文API文档示例代码

tiancaishaonvjituizi commented 2 years ago

@Ligoml 本 PR 中的 review 意见已修复,中文文档稍后提交

tiancaishaonvjituizi commented 2 years ago

@tiancaishaonvjituizi 需要在paddle/docs下同步添加中文文档哈,这里有一些参考文档:API书写规范文档预览工具中文API文档复制英文API文档示例代码

@Ligoml 好的,已提交 https://github.com/PaddlePaddle/docs/pull/4806

Ligoml commented 1 year ago
image

请merge下最新的develop分支重新提交

TCChenlong commented 1 year ago

@tiancaishaonvjituizi 请解决一下文件冲突

tiancaishaonvjituizi commented 1 year ago

@zhouwei25 https://github.com/PaddlePaddle/Paddle/pull/42469#discussion_r883487369 这个可以麻烦看一下吗

zhwesky2010 commented 1 year ago

@zhouwei25 #42469 (comment) 这个可以麻烦看一下吗

看报错有可能是不在静态图下跑导致的

tiancaishaonvjituizi commented 1 year ago

看报错有可能是不在静态图下跑导致的

😂 我不了解 OpTest 内部的实现,但按道理来说它作为基类不应该保证以静态图模式运行测试吗

对了,最新的报错有一些变化,不知道是不是只是措辞上的变动:

2022-06-08 17:12:16 PreconditionNotMetError: Tensor's dimension is out of bound.Tensor's dimension must be equal or less than the size of its memory.But received Tensor's dimension is d800 memory's size is 0.
2022-06-08 17:12:16   [Hint: Expected numel() * SizeOf(dtype()) <= memory_size(), but received numel() * SizeOf(dtype()):800 > memory_size():0.] (at /workspace/Paddle/paddle/phi/core/dense_tensor_impl.cc:58)
2022-06-08 17:12:16   [operator < nan_to_num > error]
zhwesky2010 commented 1 year ago

看报错有可能是不在静态图下跑导致的

😂 我不了解 OpTest 内部的实现,但按道理来说它作为基类不应该保证以静态图模式运行测试吗

对了,最新的报错有一些变化,不知道是不是只是措辞上的变动:

2022-06-08 17:12:16 PreconditionNotMetError: Tensor's dimension is out of bound.Tensor's dimension must be equal or less than the size of its memory.But received Tensor's dimension is d800 memory's size is 0.
2022-06-08 17:12:16   [Hint: Expected numel() * SizeOf(dtype()) <= memory_size(), but received numel() * SizeOf(dtype()):800 > memory_size():0.] (at /workspace/Paddle/paddle/phi/core/dense_tensor_impl.cc:58)
2022-06-08 17:12:16   [operator < nan_to_num > error]

和之前报错不同,看起来已经跑到OP里面去了,这个是OP内部实现的shape报错了。

ctest里会保证OPTEST在静态图跑,但是有时候是动静态图case都有,动态图case可能把模式切换了,这个时候就需要手动调整了。

tiancaishaonvjituizi commented 1 year ago

和之前报错不同,看起来已经跑到OP里面去了,这个是OP内部实现的shape报错了。

不过报错信息说的事情看起来是类似的,一个是 tensor 没有初始化,一个是 tensor 的 memory_size 为 0

ctest里会保证OPTEST在静态图跑,但是有时候是动静态图case都有,动态图case可能把模式切换了,这个时候就需要手动调整了。

感觉目前的问题不太像这个情况?目前非 OpTest 的测试用例虽然有测动态图,但既然 OpTest 会保证在静态图跑,且 OpTest 测试用例内部并没有切换动静态图,就不需要手动调整才对

zhwesky2010 commented 1 year ago

ctest只是初始时调整为静态图,中间有动态图case就变了不会再切回来

tiancaishaonvjituizi commented 1 year ago

ctest只是初始时调整为静态图,中间有动态图case就变了不会再切回来

这有点奇怪。。我的理解是 OpTest 这个基类有能力、从直觉上也应该保证在自己的作用域内以静态图运行代码,现在 develop 分支在 gcc9 上又编译不通过了,有空时我看下吧

zhwesky2010 commented 1 year ago

ctest只是初始时调整为静态图,中间有动态图case就变了不会再切回来

这有点奇怪。。我的理解是 OpTest 这个基类有能力、从直觉上也应该保证在自己的作用域内以静态图运行代码,现在 develop 分支在 gcc9 上又编译不通过了,有空时我看下吧

具体是在ctest run时,会切换为paddle.enable_static(),OpTest不会做其他调整。因此在单测里自行灵活做paddle.enable_static()、paddle.disable_static(),目前大部分单测都是这样做的

tiancaishaonvjituizi commented 1 year ago

具体是在ctest run时,会切换为paddle.enable_static(),OpTest不会做其他调整。因此在单测里自行灵活做paddle.enable_static()、paddle.disable_static()

用户在 OpTest 测试用例里是不会调用任何 paddle op 的,paddle op 是由 OpTest 内部代码调用的,而且如上面交流里所说,这些 OpTest 内部代码又只能在静态图模式下工作。所以这样一来唯一合法的选择是在 OpTest 运行前 enable_static()、在 OpTest 运行结束后恢复原始的模式。那为什么不把这一逻辑写在 OpTest 类内部,而要用户来保证呢

tiancaishaonvjituizi commented 1 year ago

在每个 OpTest 里手动 paddle.enable_static() 就修复了问题。但我并不清楚原因,因为每一个测试结束时应该都是 enable_static 的状态。

上面的建议还是希望 paddle 团队考虑下。y1s1 在这次整个开发过程中见到太多 paddle 的奇怪设计了(

Ligoml commented 1 year ago

在每个 OpTest 里手动 paddle.enable_static() 就修复了问题。但我并不清楚原因,因为每一个测试结束时应该都是 enable_static 的状态。

上面的建议还是希望 paddle 团队考虑下。y1s1 在这次整个开发过程中见到太多 paddle 的奇怪设计了(

收到,你可以把这些反馈写在黑客松活动的调研问卷里(已加微信私发过),或者直接提 issue,我们会逐一讨论并及时和你同步结论的哈

tiancaishaonvjituizi commented 1 year ago

前反向分别拆成 nan_to_num_kernel.cc/nan_to_num_kernel.cu/nan_to_num_kernel.h/nan_to_num_kernel_impl.h 和 nan_to_num_grad_kernel.cc/nan_to_num_grad_kernel.cu/nan_to_num_grad_kernel.h/nan_to_num_grad_kernel_impl.h

好的。这样拆有什么好处呢,在我看来这只是盲目地遵守了一套八股文规则,对代码复用是有损害的,而且新增一个 op 需要在 8 个文件里跳转,这也有点让人哭笑不得,您把这八个文件的文件名一个一个列出来的时候,心里也觉得很心累吧😂

tiancaishaonvjituizi commented 1 year ago

收到,你可以把这些反馈写在黑客松活动的调研问卷里(已加微信私发过),或者直接提 issue,我们会逐一讨论并及时和你同步结论的哈

好,我提了两个 issue @Ligoml https://github.com/PaddlePaddle/Paddle/issues/43700https://github.com/PaddlePaddle/Paddle/issues/43701

tiancaishaonvjituizi commented 1 year ago

@zhouwei25 已修改

Ligoml commented 1 year ago

@tiancaishaonvjituizi 请处理一下冲突和CI哈

tiancaishaonvjituizi commented 1 year ago

需要一个有权限的小伙伴重跑 rocm-compile ci @Ligoml

jeff41404 commented 1 year ago

in RFC paddle.nan_to_num should be add parameter of name , keep same with code here.

tiancaishaonvjituizi commented 1 year ago

in RFC paddle.nan_to_num should be add parameter of name , keep same with code here.

已更新

jeff41404 commented 1 year ago

use paddle.where to replace nan/posinf/neginf in python function without implement C++ API and kernel. I confirmed with relevant internal colleague in recent days, because this PR was approved before deadline, so it will not be affected by the end time of the previous hackathon, so bonuses still vaild after PR merge. Don't worry beyond the limit of hackathon, just follow the right path, I support you:)

If we all agree on this way, let's modify rfc and code, don't worry.

tiancaishaonvjituizi commented 1 year ago

I confirmed with relevant internal colleague in recent days, because this PR was approved before deadline, so it will not be affected by the end time of the previous hackathon, so bonuses still vaild after PR merge. Don't worry beyond the limit of hackathon, just follow the right path, I support you:)

这件事我和您的理解不一样,我有话直说哈 😂 我按照贵同事发布的 hackathon issue 写了 RFC 并实现了功能,在 hackathon 结束前得到了 approve。现在 hackathon 已经结束,如果您需要我做一些 PR 合并前的收尾工作我可以接受,但把原来的实现删掉,用 Python 重新实现,这已经不是收尾而是重新开发了。因为贵同事的错误,使我需要重新开发,我认为这是不合理的。虽然我看了一下重新开发的开发量并不大,但从态度上我觉得应该明确,这个需求是由于贵同事的错误引起的,提出的时机也不合理,因此我重新开发是情分,不重新开发也是已经尽了本分。所以我认为应该是这样的,看您觉得是否合理:

  1. 这个重新开发的需求不应 block PR 合并。也就是这个 PR 按现在的样子合并,hackathon 奖金正常发放;
  2. 在我有空时提交另一个 PR 进行用 Python 拼 OP 的修改(比如在做第三期 hackathon 任务的时候顺手做一下),但和奖金发放无关。
zhwesky2010 commented 1 year ago

你好,基于目前的框架新API开发要求,Python API与C++ API参数需保持一致,所以两个bool参数建议通过其他替代方式实现。

这个算子比较特殊,在框架中目前还没有这样的optional\<float> 的开发情况,如保持原方案主体不变,可考虑以下两种实现方法:

  1. 对C++参数posinf、neginf 改成 std::vector<float>,参考https://github.com/PaddlePaddle/Paddle/blob/develop/paddle/fluid/operators/set_value_op.cc#L130 ,None的情况下传递[]到C++层,在C++层判断std::vector<float> posinf是否为空来处理None的情形
  2. 对C++参数posinf、neginf改成 optional<Tensor> ,float处理为shape为[1]的Tensor传入,None直接传入,参考 https://github.com/PaddlePaddle/Paddle/blob/develop/paddle/phi/api/yaml/sparse_api.yaml#L292
tiancaishaonvjituizi commented 1 year ago
  1. 对C++参数posinf、neginf 改成 std::vector,参考https://github.com/PaddlePaddle/Paddle/blob/develop/paddle/fluid/operators/set_value_op.cc#L130 ,None的情况下传递[]到C++层,在C++层判断std::vector posinf是否为空来处理None的情形
  2. 对C++参数posinf、neginf改成 optional ,float处理为shape为[1]的Tensor传入,None直接传入,参考 https://github.com/PaddlePaddle/Paddle/blob/develop/paddle/phi/api/yaml/sparse_api.yaml#L292

这两种方式是不是都是“以辞害意”,为了遵守 “Python API与C++ API参数需保持一致”的规则,反而得到了一个不伦不类的 API (cc @zhouwei25 @jeff41404 @chenwhql )

但我会搞一下的,就当做是恰饭吧

tiancaishaonvjituizi commented 1 year ago

@zhouwei25 ,谢谢你给了详细的参考链接,对我帮助很大,不过我最终还是无法成功实现。

我试着采用第一种办法,但遇到报错

Traceback (most recent call last):
  File "/home/dev/files/repos/Paddle/paddle/fluid/eager/auto_code_generator/final_state_generator/python_c_gen.py", line 538, in <module>
    py_c_generator.run()
  File "/home/dev/files/repos/Paddle/paddle/fluid/eager/auto_code_generator/final_state_generator/python_c_gen.py", line 485, in run
    self.GeneratePythonCFunctions()
  File "/home/dev/files/repos/Paddle/paddle/fluid/eager/auto_code_generator/final_state_generator/python_c_gen.py", line 461, in GeneratePythonCFunctions
    status = f_generator.run()
  File "/home/dev/files/repos/Paddle/paddle/fluid/eager/auto_code_generator/final_state_generator/python_c_gen.py", line 427, in run
    self.CollectOriginalForwardInfo()
  File "/home/dev/files/repos/Paddle/paddle/fluid/eager/auto_code_generator/final_state_generator/codegen_utils.py", line 427, in CollectOriginalForwardInfo
    self.orig_forward_inputs_list, self.orig_forward_attrs_list, self.orig_forward_returns_list = ParseYamlForward(
  File "/home/dev/files/repos/Paddle/paddle/fluid/eager/auto_code_generator/final_state_generator/codegen_utils.py", line 307, in ParseYamlForward
    inputs_list, attrs_list = ParseYamlArgs(args_str)
  File "/home/dev/files/repos/Paddle/paddle/fluid/eager/auto_code_generator/final_state_generator/codegen_utils.py", line 229, in ParseYamlArgs
    assert arg_type in yaml_types_mapping.keys(
AssertionError: The argument type float[] in yaml config is not supported in yaml_types_mapping.

我往 yaml_types_mapping 添加了 "float[]" 之后,重新编译又遇到报错:

/home/dev/files/repos/Paddle/paddle/fluid/eager/api/generated/eager_generated/forwards/dygraph_functions.cc: In function ‘paddle::experimental::Tensor nan_to_num_final_state_dygraph_function(const paddle::experimental::Tensor&, float, std::vector<float>, std::vector<float>)’:
/home/dev/files/repos/Paddle/paddle/fluid/eager/api/generated/eager_generated/forwards/dygraph_functions.cc:9389:80: error: too many arguments to function ‘paddle::experimental::Tensor paddle::experimental::nan_to_num(const paddle::experimental::Tensor&, float)’
 9389 |   auto api_result = paddle::experimental::nan_to_num(input, nan, posinf, neginf);
      |                                                                                ^
In file included from /home/dev/files/repos/Paddle/paddle/phi/api/all.h:28,
                 from /home/dev/files/repos/Paddle/paddle/fluid/eager/grad_node_info.h:22,
                 from /home/dev/files/repos/Paddle/paddle/fluid/eager/autograd_meta.h:18,
                 from /home/dev/files/repos/Paddle/paddle/fluid/eager/api/generated/eager_generated/forwards/dygraph_functions.h:4,
                 from /home/dev/files/repos/Paddle/paddle/fluid/eager/api/generated/eager_generated/forwards/dygraph_functions.cc:3:
/home/dev/files/repos/Paddle/paddle/phi/api/include/api.h:354:19: note: declared here
  354 | PADDLE_API Tensor nan_to_num(const Tensor& input, float nan);

说明目前 paddle 的 yaml codegen 除了不支持 optional[float] 之外,也不支持 float[],而且不是简单能解决的。

然后我又尝试了 @jeff41404 的建议,试着用 paddle.where 拼装 nan_to_num,但接着发现这是不可行的,原因是 nan_to_num 需要对 +inf 和 -inf 分别处理,而 paddle.isinf 不能区分 +inf 和 -inf。我转而尝试 x == float('inf') 和 x == float('-inf'),也失败了,甚至有这样的现象:

>>> import paddle
>>> x=paddle.to_tensor([1,2,float('inf')])
>>> x==x
Tensor(shape=[3], dtype=bool, place=Place(gpu:0), stop_gradient=True,
       [True , True , False])

这表示在 paddle 里,inf == inf 是 false,这和 IEEE754 以及 Python 和 C++ 语言的实现都不一致,也和 PyTorch 不一致:

Python:

>>> float('inf')==float('inf')
True

C++:

$ cat test.cpp
#include <iostream>
#include <limits>

int main() {
  std::cout << (std::numeric_limits<float>::infinity ==
                std::numeric_limits<float>::infinity)
            << std::endl;
}
$ g++ test.cpp
$ ./a.out
1

PyTorch:

>>> import torch
>>> x=torch.tensor([1,2,float('inf')])
>>> x==x
tensor([True, True, True])

经过这两次失败之后,我本想尝试最后一种方案:用 optional<Tensor> 来表示 posinf 和 neginf。但我又重新回味了这个需求的出发点:

In order to reduce the threshold for users to use C++ API, it is necessary to ensure that the experience of using C++ API is the same with Python, that is, the parameters of the two should be the same (except name, and the different ones are being modified in plan), and the documents of Python API can be reused without writing another document about C++ API. Therefore, in this case, the parameter replace_posinf_with_max and replace_neginf_with_min should be removed. So the parameters of Python API, C++ API and kernel are the same.

很明显出发点是想让 Python API 和 C++ API 完全一样,以使得 Python 程序和 C++ 程序能够平滑的迁移,而不是只让参数个数一样,哪个用户会关心参数个数一不一样呢?因此不管是在 Python 层做 float 到 float[] 的转换还是 float 到 optional\<Tensor> 的转换,都是和出发点不符的,Python API 和 C++ API 仍然是无法平滑迁移的(Python 层是 float,C++ 层是 float[] 或 Tensor),而 OP 拼装的方案也因为 Paddle 的 bug 无法实现。

总结一下情况就是:

  1. 根据 @jeff41404 @zhouwei25 的建议,我尝试了用不同的两种方式来对齐,但都因为 Paddle 的 bug 失败了
  2. 即使没有 bug,不管是在 Python API 里做 float 到 float[] 的转换还是 float 到 Tensor 的转换,也都是不能满足 API 对齐的初衷 —— 平滑迁移 Python 和 C++ 程序的

所以我不认为应该在此时继续修改这个 API,也建议 Paddle 团队不要在基础设施不完善的情况下盲目的推行强制性的政策并带来困扰。 应该根据现实反馈调整开发规范同时尽快支持 optional\<float>。

tiancaishaonvjituizi commented 1 year ago

cc @jeff41404 @Ligoml @TCChenlong @chenwhql 请看下上面的回复

zhwesky2010 commented 1 year ago

@tiancaishaonvjituizi 你好,底层机制的问题适配起来时间会比较长,这个如果开展也会是统一调整,不阻塞当前算子工作。目前看只能通过这两种方式实现。上面的报错原因是map里没有float[]类型,目前已经修复了:

https://github.com/PaddlePaddle/Paddle/blob/develop/paddle/fluid/eager/auto_code_generator/final_state_generator/codegen_utils.py#L48

tiancaishaonvjituizi commented 1 year ago

@tiancaishaonvjituizi 你好,底层机制的问题适配起来时间会比较长,这个如果开展也会是统一调整,不阻塞当前算子工作。目前看只能通过这两种方式实现。上面的报错原因是map里没有float[]类型,其他算子已经加进来了:

但这样改有意义吗,仍然没有做到 C++ API 和 Python API 的对齐,用户还是不能平滑的迁移他的 Python 程序到 C++,仍然会很奇怪:“为什么我在 Python 里可以写 posinf=5,而在 C++ 里写 posinf=5 却会报错,只有写 posinf={5} 才行”。

目前看只能通过这两种方式实现

我的意思是我们并不需要 100% 遵守这个所谓规范。目前的情况是因为这个规范出了错引起的(它没有考虑到 optional<float> 这一出),需要修改的是规范而不是我们。而且退一步来说,规范规定的也是 “C++ 和 Python API 参数一致”,而不是 “C++ 和 Python API 参数个数一致”。不管是 float[] 还是 bool,都是一样的不一致,没有优劣之分。

tiancaishaonvjituizi commented 1 year ago

@jeff41404 可以解答一下吗,毕竟需求是您提出来的。

我认为如果一定要遵守规范(即明知道规范有漏洞,也要无脑遵守),那唯一的做法是用 paddle.where 来拼装,这样不引入新算子,也就消除了对不对齐的问题。但这个做法因为 Paddle 的 inf bug 而不可行。剩下的两种做法:

  1. C++ API 接受 float[],Python API 接受 float,在 Python API 的实现中做 float 到 float[] 的转换
  2. C++ 层接受 Tensor,Python API 接受 float,在 Python API 的实现中构造 Tensor 并做 float 到 Tensor 的转换

也都是违反了 “Python API 和 C++ API 参数一致” 的规范的,仍然没有降低用户的使用门槛。

所以如果想同时满足

  1. 让 C++ API 和 Python API 参数一致
  2. 不去支持 optional\<float>,也不去修复 inf 的 bug

这两点的话,那只能说没办法做到,所有的路都被堵死了,巧妇难为无米之炊。

所以我的建议还是这个 PR 以现在的状态合并,因为如上所述,没有更好的办法了。就算现在改成了 float[],既没有变得符合规范,而且等不久之后支持了 optional\<float>,又要再改一次,有什么必要呢

zhwesky2010 commented 1 year ago

@jeff41404 可以解答一下吗,毕竟需求是您提出来的。

我认为如果一定要遵守规范(即明知道规范有漏洞,也要无脑遵守),那唯一的做法是用 paddle.where 来拼装,这样不引入新算子,也就消除了对不对齐的问题。但这个做法因为 Paddle 的 inf bug 而不可行。剩下的两种做法:

  1. C++ API 接受 float[],Python API 接受 float,在 Python API 的实现中做 float 到 float[] 的转换
  2. C++ 层接受 Tensor,Python API 接受 float,在 Python API 的实现中构造 Tensor 并做 float 到 Tensor 的转换

也都是违反了 “Python API 和 C++ API 参数一致” 的规范的,仍然没有降低用户的使用门槛。

所以如果想同时满足

  1. 让 C++ API 和 Python API 参数一致
  2. 不去支持 optional,也不去修复 inf 的 bug

这两点的话,那只能说没办法做到,所有的路都被堵死了,巧妇难为无米之炊。

所以我的建议还是这个 PR 以现在的状态合并,因为如上所述,没有更好的办法了。就算现在改成了 float[],既没有变得符合规范,而且等不久之后支持了 optional,又要再改一次,有什么必要呢

嗯如果不赞同C++的当前实现方案,那建议就还是由paddle.where组装吧,这个inf的API bug目前已经通知去修复了

tiancaishaonvjituizi commented 1 year ago

嗯如果不赞同C++的当前实现方案,那建议就还是由paddle.where组装吧,这个inf的API bug目前已经通知去修复了

嗯,那就先等待修复吧

zhwesky2010 commented 1 year ago

嗯如果不赞同C++的当前实现方案,那建议就还是由paddle.where组装吧,这个inf的API bug目前已经通知去修复了

嗯,那就先等待修复吧

equal判断 inf==inf 的API bug已修复,麻烦看看还有没有其他问题

tiancaishaonvjituizi commented 1 year ago

嗯如果不赞同C++的当前实现方案,那建议就还是由paddle.where组装吧,这个inf的API bug目前已经通知去修复了

嗯,那就先等待修复吧

equal判断 inf==inf 的API bug已修复,麻烦看看还有没有其他问题

真的还有(哭笑不得),用 np.finfo(np.float64).max 给 paddle float64 tensor 赋值变成了 inf:

>>> x=torch.empty(2, dtype=torch.float64)
>>> torch.full_like(x, np.finfo(np.float64).max)
tensor([1.7977e+308, 1.7977e+308], dtype=torch.float64)
>>> x=paddle.to_tensor([1,1], dtype=paddle.float64)
>>> paddle.full_like(x, np.finfo(np.float64).max)
Tensor(shape=[2], dtype=float64, place=Place(gpu:0), stop_gradient=True,
       [inf., inf.])

可能是哪里的中间变量数据类型被误用了 float32 而不是 float64,因为我观察到下面的现象:

>>> paddle.full_like(x, np.finfo(np.float32).max)
Tensor(shape=[2], dtype=float64, place=Place(gpu:0), stop_gradient=True,
       [340282346638528859811704183484516925440.,
        340282346638528859811704183484516925440.])
>>> paddle.full_like(x, np.finfo(np.float32).max*2)
Tensor(shape=[2], dtype=float64, place=Place(gpu:0), stop_gradient=True,
       [inf., inf.])

现在 nan_to_num float32 的测试已经过了,代码已上传。有一说一,我觉得我真的仁至义尽了。一个简简单单的 api,approve 后又一刀切要求做巧妇难为无米之炊的重写,接着经过反复拉扯终于确定了可行的重写方案,又接二连三遇到 paddle 的各种 bug。要不您跟您的领导反映一下,我已经把这个 api 基本实现了,剩下的问题只有 paddle 自己的 bug,您或者您的领导 @jeff41404 @zhouwei25 自己继续推动这个 api 吧,我仁至义尽了,一开始就表达过,我现在在做的事情是情分不是本分

zhwesky2010 commented 1 year ago

OK,我们来处理这些问题

zhwesky2010 commented 1 year ago

@tiancaishaonvjituizi 你好,我们正在开展OP attribute不支持double类型的工作,但由于涉及到底层OP架构修改和API/OP的不兼容升级改动,时间跨度可能较长。经过内部讨论,当前nan_to_num API开发先暂时绕过,支持float32类型的单测即可,float64单测case先写上但是先注释掉就可以。

tiancaishaonvjituizi commented 1 year ago

@zhouwei25 ok 了,留了一个 float32 的测试

Ligoml commented 1 year ago
image

重新跑一下pre-commit吧

tiancaishaonvjituizi commented 1 year ago

重新跑一下pre-commit吧

好(

tiancaishaonvjituizi commented 1 year ago

@Ligoml 该怎么让 pre-commit 重新安装格式化工具呢?我这里的 yapf 版本似乎和 CI 里用的 yapf 版本不同,得不到和 CI 一样的格式化结果,我暂时按照 CI 输出的 diff 手动格式化了

luotao1 commented 1 year ago
2022-10-24 15:51:31 Your PR code style check failed.
2022-10-24 15:51:31 Please install pre-commit locally and set up git hook scripts:
2022-10-24 15:51:31 
2022-10-24 15:51:31     pip install pre-commit==2.17.0
2022-10-24 15:51:31     pre-commit install
2022-10-24 15:51:31 
2022-10-24 15:51:31 Then, run pre-commit to check codestyle issues in your PR:
2022-10-24 15:51:31 
2022-10-24 15:51:31     pre-commit run --files python/paddle/__init__.py python/paddle/fluid/tests/unittests/test_nan_to_num_op.py python/paddle/tensor/__init__.py python/paddle/tensor/math.py
2022-10-24 15:51:31 

我这里的 yapf 版本似乎和 CI 里用的 yapf 版本

develop分支于昨天刚刚从yapf版本升级到black版本。需要先merge下develop分支,然后按上面的步骤操作下,会自动格式化的。