[Cpp API Compatibility] add at::vstack compat interface - #79175
Conversation
|
你的PR提交成功，感谢你对开源项目的贡献! |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds an ATen compatibility implementation of vstack and validates it with new C++ tests.
Changes:
- Introduced
at::vstack(const std::vector<at::Tensor>&)compatibility API implemented via preprocessing +at::cat. - Added C++ unit tests covering basic shape/dtype behavior and empty input handling.
- Exported the new op through
compat/ATen/Functions.h.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| test/cpp/compat/ATen_vstack_test.cc | Adds unit tests for at::vstack across shapes/dtypes and an empty-list error case. |
| paddle/phi/api/include/compat/ATen/ops/vstack.h | Implements at::vstack for compat by normalizing tensor ranks then concatenating. |
| paddle/phi/api/include/compat/ATen/Functions.h | Includes the new vstack op header in the compat Functions umbrella header. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #include <ATen/core/Tensor.h> | ||
| #include <c10/util/Exception.h> | ||
| #include <vector> | ||
|
|
||
| #include "paddle/phi/api/include/api.h" | ||
|
|
||
| namespace at { | ||
|
|
||
| inline at::Tensor vstack(const std::vector<at::Tensor>& tensors) { | ||
| TORCH_CHECK(!tensors.empty(), "vstack expects a non-empty TensorList"); | ||
|
|
||
| std::vector<at::Tensor> processed; | ||
| processed.reserve(tensors.size()); | ||
|
|
||
| for (const auto& t : tensors) { | ||
| if (t.dim() == 0) { | ||
| processed.push_back(t.reshape({1, 1})); | ||
| } else if (t.dim() == 1) { | ||
| processed.push_back(t.unsqueeze(0)); | ||
| } else { | ||
| processed.push_back(t); | ||
| } | ||
| } | ||
|
|
||
| return at::cat(processed, 0); | ||
| } |
| #include <ATen/core/TensorBody.h> | ||
| #include <ATen/ops/vstack.h> | ||
| #include <c10/core/ScalarType.h> | ||
| #include <c10/core/TensorOptions.h> |
| TEST(ATenVStackTest, Basic2D) { | ||
| auto t1 = at::ones({2, 3}, at::kFloat); | ||
| auto t2 = at::zeros({2, 3}, at::kFloat); | ||
| std::vector<at::Tensor> tensors = {t1, t2}; | ||
| auto result = at::vstack(tensors); | ||
|
|
||
| EXPECT_EQ(result.dim(), 2); | ||
| EXPECT_EQ(result.size(0), 4); | ||
| EXPECT_EQ(result.size(1), 3); | ||
| } |
|
|
||
| TEST(ATenVStackTest, EmptyListThrows) { | ||
| std::vector<at::Tensor> tensors = {}; | ||
| ASSERT_THROW(at::vstack(tensors), std::exception); |
risemeup1111
left a comment
risemeup1111
left a comment
There was a problem hiding this comment.
已完成首轮 review。当前还有需要修复后再合入的问题，具体见行级评论：vstack.h 需要补齐自包含依赖，新增 ATen_vstack_test.cc 也需要接入 CMake，确保 CI 能编译和运行这些用例。
非行级：当前 Check PR Template 失败，日志显示 PR Category、PR Types 和“是否引起精度变化”字段未按模板填写。请按仓库 PR 模板补齐这些字段，否则 CI 会继续阻塞。
| #include "gtest/gtest.h" | ||
| #include "torch/all.h" | ||
|
|
||
| TEST(ATenVStackTest, Basic2D) { |
There was a problem hiding this comment.
这个新增测试文件还没有接入 test/cpp/compat/CMakeLists.txt。该目录是手动列举 cc_test(...) 目标的，当前 rg ATen_vstack_test test/cpp/compat/CMakeLists.txt 没有结果，所以 CI/ctest 不会编译或运行这里的用例，PR 描述里的 ctest ATen_vstack_test 在当前提交也不可复现。请把测试目标加入 CMake，例如：
cc_test(ATen_vstack_test SRCS ATen_vstack_test.cc)
1. vstack.h: Add self-contained includes (cat.h, reshape.h, unsqueeze.h) 2. ATen_vstack_test.cc: Add explicit <vector> include, value assertions (allclose/equal checks for slice correctness), keep std::exception for ASSERT_THROW as c10::Error is unavailable in compat layer. 3. CMakeLists.txt: Register ATen_vstack_test target. Review fixes: - PaddlePaddle#79175 (comment) - PaddlePaddle#79175 (comment) - PaddlePaddle#79175 (comment) - PaddlePaddle#79175 (comment) - PaddlePaddle#79175 (review) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
risemeup1111
left a comment
risemeup1111
left a comment
There was a problem hiding this comment.
已复查当前提交。上轮提出的两个阻塞点均已处理：vstack.h 已补齐自包含依赖，ATen_vstack_test 也已接入 CMake；未发现新的需要阻塞合入的代码问题。
当前 Check approval 仍提示新增 cc_test 需要对应 RD 审批，这属于仓库审批规则，请以后续 CI 状态为准。
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #79175 +/- ##
===========================================
Coverage ? 100.00%
===========================================
Files ? 1
Lines ? 11
Branches ? 0
===========================================
Hits ? 11
Misses ? 0
Partials ? 0 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
risemeup1111
left a comment
risemeup1111
left a comment
There was a problem hiding this comment.
已复查最新提交。此次变更仅移除了 vstack.h 中未使用的 include，此前关于头文件自包含和测试接入 CMake 的问题仍保持解决；未发现新的需要阻塞合入的代码问题。
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
CI报告基于以下代码生成（30分钟更新一次）: 1 Required任务 : 42/48 通过
2 失败详情🔴 Coverage build — 环境问题（置信度: 高）错误类型: 环境问题 | 置信度: 高
关键日志:
修复建议:
关联变更: PR 修改 ATen 兼容接口、C++ compat 单测和静态检查脚本；本 job 未进入源码构建阶段。 🔴 Distribute-stable-build / Build — 环境问题（置信度: 高）错误类型: 环境问题 | 置信度: 高
关键日志:
修复建议:
关联变更: PR 变更未进入该 job 的构建验证阶段，暂无证据指向源码问题。 🔴 Check approval — 需要 Approval（置信度: 高）错误类型: 需要 Approval | 置信度: 高 关键日志:
修复建议:
关联变更: 无 🟠 Slice / Slice test — 不稳定问题（置信度: 中）错误类型: 不稳定问题 | 置信度: 中
关键日志:
修复建议:
关联变更: |
|
/re-run all-failed |
1 similar comment
|
/re-run all-failed |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
1. vstack.h: Add self-contained includes (cat.h, reshape.h, unsqueeze.h) 2. ATen_vstack_test.cc: Add explicit <vector> include, value assertions (allclose/equal checks for slice correctness), keep std::exception for ASSERT_THROW as c10::Error is unavailable in compat layer. 3. CMakeLists.txt: Register ATen_vstack_test target. Review fixes: - PaddlePaddle#79175 (comment) - PaddlePaddle#79175 (comment) - PaddlePaddle#79175 (comment) - PaddlePaddle#79175 (comment) - PaddlePaddle#79175 (review) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
522c89e to
3ac1171
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
6b126af to
e808ab0
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
risemeup1111
left a comment
risemeup1111
left a comment
There was a problem hiding this comment.
已复查当前提交，未发现需要阻塞合入的代码问题。vstack.h 已保持自包含，ATen_vstack_test 已接入 CMake，新增接口签名也与 libtorch 的 at::vstack(at::TensorList) 保持一致。
当前 Check approval 仍显示需要审批，请以后续 CI/审批状态为准。
CI报告基于以下代码生成（30分钟更新一次）: 1 Required任务 : 41/43 通过
2 失败详情🔴 Coverage test — 环境问题（置信度: 高）分析器: 通用分析(fallback) 失败用例: 无（测试阶段未执行）
关键日志:
修复建议:
关联变更: PR 仅修改 🔴 Check approval — 需要 Approval（置信度: 高）该 Job 需要人工 Approval，完成审批后 CI 才会继续执行。
修复建议:
关联变更: 与 PR 代码变更无关。 |
e808ab0 to
2963892
Compare
risemeup1111
left a comment
risemeup1111
left a comment
There was a problem hiding this comment.
已复查新提交，未发现需要阻塞合入的代码问题；此前关于 vstack.h 自包含和 ATen_vstack_test 接入 CMake 的问题仍保持解决。新增变更主要是在 test/cpp/fluid/fused/CMakeLists.txt 中补充若干 GPU fused 测试目标的链接依赖。
优先级：P3 非行级：PR 描述当前只说明
at::vstack兼容接口和 compat 单测，但 diff 还新增了test/cpp/fluid/fused/CMakeLists.txt中conditional_block_op/executor依赖调整。请在 Description 里补一条类似“补充 fused GPU 测试目标的链接依赖以修复相关测试构建”的说明；如果这不是本 PR 必需的 CI 修复，建议拆到独立 PR，避免兼容接口变更和无关测试构建修复混在一起。
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
/re-run all-failed |
2963892 to
b17c01c
Compare
liuhao2638
left a comment
liuhao2638
left a comment
There was a problem hiding this comment.
已复查当前提交，上一轮提到的额外 fused CMake 依赖变更已从 PR diff 中移除；当前变更范围回到 at::vstack 兼容接口和对应 compat 单测。此前关于 vstack.h 自包含、ATen_vstack_test 接入 CMake 的问题仍保持解决，未发现新的需要阻塞合入的代码问题。
当前 Check approval 仍显示需要审批，请以后续 CI/审批状态为准。
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
/re-run all-failed |
b17c01c to
7b53e0b
Compare
PaddlePaddle-bot
left a comment
PaddlePaddle-bot
left a comment
There was a problem hiding this comment.
🤖 Paddle-CI-Agent | pr_review |
2026-07-08 20:10:24
📋 Review 摘要
PR 概述：新增 at::vstack C++ ATen 兼容接口，通过 0D/1D 预处理后复用 at::cat 沿 dim 0 拼接
变更范围：paddle/phi/api/include/compat/ATen/、test/cpp/compat/
影响面 Tag：[Execute Infrastructure] [User Experience]
问题
| 级别 | 文件 | 概述 |
|---|---|---|
| 🟡 建议 | test/cpp/compat/ATen_vstack_test.cc:139 |
PR 描述声称覆盖的 50x100、零维度、全一维度用例未在新增 C++ 测试中落地 |
📝 PR 规范检查
当前标题 Tag [Cpp API Compatibility] 不在 Paddle PR 模板枚举中，建议使用与本 PR Category 一致的官方 Tag。描述结构包含必填 section，可一并按下面内容收敛为可验证范围。
标题建议（可直接复制）：
[Execute Infrastructure] Add at::vstack compat interface
PR 描述建议（点击展开，可直接复制）
### PR Category
Execute Infrastructure
### PR Types
New features
### Description
为 Paddle C++ ATen 兼容层新增 `at::vstack` 接口，用于将 TensorList 中的张量按 PyTorch vstack 语义转换后沿第 0 维拼接：
- 0D 张量 reshape 为 `(1, 1)`
- 1D 张量 unsqueeze(0) 为 `(1, N)`
- 2D+ 张量保持原 shape 并沿 dim 0 concat
- 空 TensorList 通过 `TORCH_CHECK` 抛出异常
主要变更：
- 新增 `paddle/phi/api/include/compat/ATen/ops/vstack.h`
- 在 `paddle/phi/api/include/compat/ATen/Functions.h` 导出 `ATen/ops/vstack.h`
- 在 `paddle/phi/api/include/compat/ATen/core/TensorBody.h` 补充 `c10::ArrayRef` include 和 `TensorList` alias
- 新增 `test/cpp/compat/ATen_vstack_test.cc` 并加入 `test/cpp/compat/CMakeLists.txt`
验证：
- 新增 C++ compat 单测覆盖 0D、1D、2D、1D+2D 混合、空输入异常，以及 `kFloat`、`kDouble`、`kInt`、`kLong` dtype。
- PCAT 跨框架对比测试见 PFCCLab/PaddleCppAPITest#65。
### 是否引起精度变化
否总体评价
核心实现路径与 PyTorch vstack 的 0D/1D 预处理语义基本一致，未发现阻塞性代码问题。建议补齐或收敛测试覆盖声明后继续推进。
| auto slice1 = result.slice(0, 2, 4); | ||
| EXPECT_TRUE(at::equal(slice0, t1)); | ||
| EXPECT_TRUE(at::equal(slice1, t2)); | ||
| } |
There was a problem hiding this comment.
🟡 建议 PR 描述声称新增 shape 覆盖包含 2D 大 shape (50, 100)、含零维度和全一维度，但当前新增的 8 个 C++ case 只覆盖标量、1D、普通 2D、1D+2D 混合、空列表和 dtype。
这会让 Review 和后续回归判断以为这些边界已经在本仓测试中守住。建议二选一：补齐对应 C++ case，例如 at::zeros({0, 3}) / at::ones({1, 1}) / {50, 100} 输入的 vstack 结果 shape 和内容断言；或者收敛 PR 描述，只声明当前实际覆盖的 case。
risemeup1111
left a comment
risemeup1111
left a comment
There was a problem hiding this comment.
已复查当前提交，变更范围仍集中在 at::vstack 兼容接口和对应 compat 单测；此前关于 vstack.h 自包含、ATen_vstack_test 接入 CMake 的问题仍保持解决，未发现新的需要阻塞合入的代码问题。
当前 Check approval 仍显示需要审批，请以后续 CI/审批状态为准。
|
/re-run all-failed |
3 similar comments
|
/re-run all-failed |
|
/re-run all-failed |
|
/re-run all-failed |
|
/re-run all-failed |
PR Category
Execute Infrastructure
PR Types
New features
Description
为 Paddle C++ 兼容层新增
at::vstack接口，用于将张量序列沿垂直方向（第 0 维）堆叠。paddle/phi/api/include/compat/ATen/ops/vstack.htest/cpp/compat/ATen_vstack_test.cc（8 个用例）PCAT 跨框架对比测试同步新增 12 个用例，Paddle / PyTorch 双端均通过，
result_cmp无差异。PR: PFCCLab/PaddleCppAPITest#65是否引起精度变化
否