[API Compatibility] Align paddle.Tensor.is_sparse, paddle.Tensor.type, paddle.Tensor.size api, paddle.nn.PReLU and paddle.distributions.categorical.Categorical -part - #79550
Conversation
|
你的PR提交成功,感谢你对开源项目的贡献! |
There was a problem hiding this comment.
| 序号 | 位置 | 优先级 | 状态 |
|---|---|---|---|
| 1 | python/paddle/utils/decorator_utils.py |
🟡 | |
| 2 | python/paddle/compat/proxy.py |
✅ | |
| 3 | python/paddle/compat/__init__.py |
✅ | |
| 4 | test_project/paddle_temp.py |
✅ |
点击展开 Review 规则
PR 评审规则:- P0、P1 级别的评审必须提交新的 commit 进行修改;
- P2、P3 级别的评审可以通过评论进行修改。
状态说明:
| 已解决 | 无需修复 | 待修复 |
|---|---|---|
| ✅ | 🟡 | 🚧 |
| is_torch_device = weight_attr is None or ( | ||
| isinstance(weight_attr, str) | ||
| and weight_attr.split(':', 1)[0].lower() | ||
| in {'cpu', 'cuda', 'gpu', 'xpu', 'mps', 'meta'} | ||
| ) | ||
| if is_torch_device: | ||
| if not isinstance(data_format, str): | ||
| device, dtype = weight_attr, data_format | ||
| weight_attr, data_format = None, "NCHW" | ||
| elif weight_attr is not None and data_format == "NCHW": | ||
| device, weight_attr = weight_attr, None |
There was a problem hiding this comment.
问题: 这里把第三个位置参数为 "cpu"/"cuda" 等字符串且 data_format 仍为默认值的调用重解释成 PyTorch 的 device。但 weight_attr: ParamAttrLike 旧 API 明确支持字符串参数名,ParamAttr._to_attr("cpu") 会创建名为 "cpu" 的参数;现在 paddle.nn.PReLU(2, 0.5, "cpu") 会丢掉 weight_attr 并创建匿名参数。
影响: 这是原生 paddle.nn.PReLU 构造函数的全局行为变化,不需要开启 enable_compat(level=2) 就会触发,会破坏依赖参数名的旧模型代码、静态图或 checkpoint/state_dict 兼容性。
处理要求:请针对该评论修复并提交新的 commit。
期望: 保留旧的字符串 weight_attr 语义;PyTorch 的无 dtype 位置参数 device 形态建议放到 paddle.compat.nn.PReLU/level=2 alias 中处理,或至少只在不会与旧 weight_attr 字符串冲突的形态下重解释。例如可以按下面的形态收窄 native 构造函数里的自动重解释:
# 伪代码:仅处理第四个位置参数明确是 dtype/device pair 的无冲突形态
if not isinstance(data_format, str):
device, dtype = weight_attr, data_format
weight_attr, data_format = None, "NCHW"
# PReLU(..., "cpu") 这种无 dtype 的 PyTorch 形态不要在 native paddle.nn.PReLU
# 中覆盖字符串 weight_attr;如需支持,请放到 compat-only wrapper 中。… three_Tensor_and_PReLU
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #79550 +/- ##
==========================================
Coverage ? 91.77%
==========================================
Files ? 6
Lines ? 158
Branches ? 0
==========================================
Hits ? 145
Misses ? 13
Partials ? 0 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
/re-run all-failed |
|
/re-run all-failed |
risemeup1111
left a comment
There was a problem hiding this comment.
已复查当前 head,先前指出的 PReLU(2, 0.5, "cpu") 后向兼容问题在代码上已经修复。本次新增一个非阻塞的行级建议,细节见 inline comment;另外 CI 里 Static-Check / Slice 仍有红灯,合入前还需要继续跟进。
| return f"{prefix}.{tensor_type}" | ||
|
|
||
| device = None | ||
| if isinstance(dtype, type): |
There was a problem hiding this comment.
问题: 这里用 isinstance(dtype, type) 捕获所有 Python type,然后只按 FloatTensor/DoubleTensor 这类 tensor factory 名称识别。DTypeLike 里包含 np.float32、np.float64、np.int64 等 numpy scalar type;这些对象也是 type,会在这里因为 dtype.__name__(例如 "float64")不在 _TENSOR_TYPE_DTYPES 而直接 ValueError,无法走后面的 input.to(dtype=dtype)。
影响: enable_compat(level=2) 后,t.type(np.float64) 这类 Paddle 其他 dtype 参数常见写法会被误判为非法 tensor class。与当前签名里的 DTypeLike | str | type 不一致,也缺少测试覆盖。
处理要求:请针对该评论进行回复(同意并已修改请回复 Done,不同意请说明理由)。
期望: 只把已知 tensor factory class 当作 PyTorch-style class conversion,其他 DTypeLike 继续交给 Tensor.to 校验/转换,并补一个 np.float64 之类的回归测试。可以先把该行收窄为:
| if isinstance(dtype, type): | |
| if isinstance(dtype, type) and dtype.__name__ in _TENSOR_TYPE_DTYPES: |
There was a problem hiding this comment.
补充复查:新提交的 test_tensor_type_edge_cases 覆盖了 t.type(paddle.float64, **{"async": True}),但这个参数不是 Python type,不会进入当前 if isinstance(dtype, type): 分支;原问题里的 np.float64 / np.int64 等 numpy scalar type 仍会进入该分支并因为 "float64" 不在 _TENSOR_TYPE_DTYPES 里抛 ValueError。
处理要求:请针对该评论进行回复(同意并已修改请回复 Done,不同意请说明理由)。
建议仍按上面 suggestion 收窄 tensor factory class 的判断,并把回归测试补到 numpy dtype 路径,例如:
import numpy as np
self.assertEqual(t.type(np.float64).dtype, paddle.float64)There was a problem hiding this comment.
补充复查:这个提交把原来的判断改成了 dtype.__name__ in _TENSOR_TYPE_NAMES,但 _TENSOR_TYPE_NAMES 的 key 是 "float32"/"float64" 这类 dtype 名;paddle.DoubleTensor 来自 dtype_tensor_factory('float64', 'DoubleTensor'),__name__ 是 "DoubleTensor"。因此 t.type(paddle.DoubleTensor) 不会进入这里的 tensor factory class 分支,会继续把 class 对象传给 input.to(dtype=...),这不是 DTypeLike 支持的输入,新增的 self.assertEqual(t.type(paddle.DoubleTensor).dtype, paddle.float64) 也会失败。
处理要求:请针对该评论修复并提交新的 commit。
请把 class 分支的 membership 查 _TENSOR_TYPE_DTYPES(反查表)而不是 _TENSOR_TYPE_NAMES,并保留这次补上的 np.float64 回归测试。修复形态可以是:
if isinstance(dtype, type) and dtype.__name__ in _TENSOR_TYPE_DTYPES:
tensor_type = dtype.__name__
dtype = _TENSOR_TYPE_DTYPES[tensor_type]
device = "cpu"
risemeup1111
left a comment
There was a problem hiding this comment.
已复查最新提交。先前的 PReLU(2, 0.5, "cpu") 后向兼容问题在当前代码里仍是修复状态。
不过 _tensor_type 的 follow-up 修复把 tensor factory class 的查表对象写反了,当前仍会影响 t.type(paddle.DoubleTensor) 这类兼容路径;我已在既有行级线程里补充了具体证据和修复形态。这个问题需要再提交一个修复 commit 后才能通过。当前还有部分 CI 在运行中,请合入前继续确认结果。
risemeup1111
left a comment
There was a problem hiding this comment.
已复查最新提交,先前关于 PReLU 后向兼容和 Tensor.type dtype/class 处理的意见在当前代码中都已修复,未发现新的需要阻塞合入的问题。当前仍有部分 CI 在运行中,合入前请继续确认最终结果。
|
/re-run all-failed |
1 similar comment
|
/re-run all-failed |
risemeup1111
left a comment
There was a problem hiding this comment.
已复查最新提交。此前关于 PReLU 和 Tensor.type 的问题在当前代码中仍是修复状态。
这次新增的 Categorical 参数校验还有一个需要修正的兼容性问题,具体见行级评论。当前仍有部分 CI 在运行中,合入前请继续确认最终结果。
|
/re-run all-failed |
risemeup1111
left a comment
There was a problem hiding this comment.
已复查你在 Categorical 线程里的说明,确认当前实现与 PyTorch 的 probs 归一化/校验顺序一致;此前这条阻塞意见已撤回。先前关于 PReLU 和 Tensor.type 的问题在当前 head 中也仍是修复状态,未发现新的需要阻塞合入的问题。
当前仍有部分 CI 未完成/有红灯,请合入前继续确认最终结果。
risemeup1111
left a comment
There was a problem hiding this comment.
已复查标题更新,当前标题已覆盖这次新增的 Categorical 兼容范围;代码 head 未变化,先前评审结论不变,未发现新的需要阻塞合入的问题。
当前仍有部分 CI 未完成/有红灯,请合入前继续确认最终结果。
|
/re-run all-failed |
|
@zhwesky20 review this PR, Thx |
|
@Manfredss 机器人这个CI能通过吗 |
我觉得是不需要改这个地方的,从最原始的版本开始(添加 level 前)就是这样实现的,且对运行没有影响 |
这个你需要和机器人沟通,直到机器人approve。不然CI是强制不让过的 |
|
/re-run all-failed |
@risemeup1111 还有什么地方没有通过? |
|
目前只剩 1 项未通过: 当前 |
@risemeup1111 两位审核已经同意此次不修改这个地方,已经显式 approve;且当前并不影响功能与使用,从最原始的版本开始(添加 level 前)就是这样实现的 |
|
你说得对。我重新对比了 base head 与当前实现:重复 |
| raise TypeError(f"type() got an unexpected keyword argument {key!r}") | ||
|
|
||
| if dtype is None: | ||
| return str(input.dtype) |
There was a problem hiding this comment.
你这个地方需要映射为torch一致的paddle.FloatTensor这种格式
There was a problem hiding this comment.
这个是 paddle.FloatTensor 还是 torch.FloatTensor? paconvert 那边比较的话是拿 paddle 的返回值和 torch 的返回值比对的,torch 返回 'torch.FloatTensor', 所以我觉得这里也用 torch 吧,但确实 paddle 里调用返回 torch 很怪
>>> import torch
>>> a = torch.tensor([2.], dtype=float32)
>>> a.type()
'torch.FloatTensor'然后像 uint16 这些直接返回字符串的,返回的也是 paddle.uint16 这样的,需要拼成 torch 前缀吗
There was a problem hiding this comment.
There was a problem hiding this comment.
我的想法是另开一个 PR 修这个问题,要涉及到修改 creation.py,并且还有是 torch 还是 paddle 前缀的问题
There was a problem hiding this comment.
我的想法是另开一个 PR 修这个问题,要涉及到修改 creation.py,并且还有是 torch 还是 paddle 前缀的问题
我认为 #79641 这样的修改是对的
There was a problem hiding this comment.
当前这项仍需在本 PR 修复。compat 模式下 t.type() 仍返回 paddle.float32,而本 PR 描述明确声明对齐 PyTorch 的 type query;CPU float32 应返回 torch.FloatTensor,GPU 与 sparse COO 还需编码 place/layout。现有测试反而将 paddle.float32 固化为预期值。请将 #79641 中已确认的类型名生成逻辑及 CPU/GPU/稀疏回归测试合入本 PR 后再合入;单独后续修复会使本 PR 先发布一个与声明不一致的公共行为。
There was a problem hiding this comment.
当前提交已修复 CPU、CUDA 和 sparse COO 的查询,但这项仍是部分修复:_tensor_type_name() 只判断 is_gpu_place(),XPU/custom place 都会返回 paddle.FloatTensor,再调用 t.type(t.type()) 时该字符串会按 CPU 类型解析并迁移设备。请像 #79641 的后续实现一样为 XPU 和 custom device 保留设备段,并补对应 fake-place/round-trip 测试;完成后这项即可关闭。
There was a problem hiding this comment.
已确认当前提交为 XPU 和已注册 custom device 保留了设备段,字符串解析与 no-op 判断也使用相同的 place 映射;新增测试覆盖了 CPU/GPU/XPU/custom 输出、有效转换和非法多设备段。此前的设备 round-trip 问题已解决。
… three_Tensor_and_PReLU
|
|
||
| import paddle | ||
|
|
||
| paddle.enable_compat(level=2) |
There was a problem hiding this comment.
已确认当前提交删除了这两个临时脚本,此项已解决。
| raise TypeError(f"type() got an unexpected keyword argument {key!r}") | ||
|
|
||
| if dtype is None: | ||
| return str(input.dtype) |
There was a problem hiding this comment.
当前提交已修复 CPU、CUDA 和 sparse COO 的查询,但这项仍是部分修复:_tensor_type_name() 只判断 is_gpu_place(),XPU/custom place 都会返回 paddle.FloatTensor,再调用 t.type(t.type()) 时该字符串会按 CPU 类型解析并迁移设备。请像 #79641 的后续实现一样为 XPU 和 custom device 保留设备段,并补对应 fake-place/round-trip 测试;完成后这项即可关闭。
| if has_paddle_aliases: | ||
| _apply_paddle_namespace_aliases() | ||
| else: | ||
| disable_compat() |
There was a problem hiding this comment.
当前提交已同步 develop 的 compat 生命周期实现:enable_compat() 对 finder 采用幂等插入,guard 也按原 level 恢复。因此此前的重复 finder/禁用不完整问题已解决,此线程可以关闭。
|
/re-run all-failed |
|
/re-run all-failed |
PR Category
User Experience
PR Types
Improvements
Description
Align the following Paddle APIs with their PyTorch counterparts:
paddle.Tensor.numel()to return a Pythonintunderenable_compat(level=2).paddle.Tensor.is_sparseas a property and returnTrueonly for sparse COO tensors.paddle.Tensor.type()behavior, including type queries, dtype/class/string conversions, device handling, and same-type identity preservation.paddle.nn.PReLUto accept PyTorch-styledeviceanddtypepositional and keyword arguments.paddle.distributions.categorical.CategoricalPaddle-internal callers retain native Tensor semantics, and all patched attributes are restored after disabling compat.
Also refine the compat level semantics and make
use_compat_guardrestore the originalcompatstate. Add lifecycle and real-torch regression tests. This is related to是否引起精度变化
否