Skip to content

[Cpp API Compatibility] Align resize api #78554

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.

Sign up for GitHub

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

Merged
SigureMo merged 3 commits into from
Apr 2, 2026
+166 −6
Merged

[Cpp API Compatibility] Align resize api #78554

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
902aa11
fix resize_
youge325 Mar 30, 2026
f027442
fix resize_ again
youge325 Mar 30, 2026
cf8a6da
fix
youge325 Apr 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 22 additions & 4 deletions paddle/phi/api/include/compat/ATen/ops/resize.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,31 @@

namespace at {

// resize_ - in-place resize using reshape
// resize_ - use reshape for same-numel cases and set_ for storage-changing
// cases so repeated resize_ calls stay stable.
inline const at::Tensor& Tensor::resize_(
at::IntArrayRef size,
::std::optional<at::MemoryFormat> memory_format) const {
auto result =
paddle::experimental::reshape(tensor_, size._PD_ToPaddleIntArray());
const_cast<Tensor*>(this)->tensor_ = result;
if (memory_format.has_value()) {
TORCH_CHECK(*memory_format == at::MemoryFormat::Contiguous,
Copy link
Copy Markdown
Contributor

@ShigureNyako ShigureNyako Apr 2, 2026

Choose a reason for hiding this comment

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

这里把 memory_format 限死为 Contiguous，和 upstream resize_ 也还没对齐。

我查了 PyTorch 的实现，resize_ 会把 memory_format 继续传给 empty_tensor_restride，也就是至少允许 Contiguous / ChannelsLast / ChannelsLast3d，只会拒绝 Preserve 这类不支持的值。当前 compat 实现则会把所有非 Contiguous 的调用直接变成异常。

这不仅是兼容性缺口，还是行为级 breaking change：这个 compat 层原来对 memory_format 参数是“静默接受但不生效”，而这版会让 channels_last 调用直接报错。既然 PR 标题和描述都在强调“Align resize api”，这里最好补到和 upstream 一致，并加一组对应的回归测试。

Copy link
Copy Markdown
Member

@SigureMo SigureMo Apr 2, 2026

Choose a reason for hiding this comment

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

嗯？相比于之前不是 breaking 吧？之前应该 Paddle 本身也没支持，会有更不友好的报错，这里报错是合理的

Copy link
Copy Markdown
Contributor

@ShigureNyako ShigureNyako Apr 2, 2026

Choose a reason for hiding this comment

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

嗯？相比于之前不是 breaking 吧？之前应该 Paddle 本身也没支持，会有更不友好的报错，这里报错是合理的

你这个提醒有道理：如果只从 Paddle 内核能力看，这里直接报错不算离谱。

我这里说的兼容风险，主要是 compat 层相对于当前已有行为的回退：旧实现这里是直接 reshape，memory_format 参数虽然没有真正生效，但在 same-numel 场景下不会在 compat 层同步抛错；这版新增 guard 之后，会把原来能走通的 channels_last / channels_last_3d 调用直接变成异常。

所以我补了一个更保守的 inline suggestion：如果这次 split PR 想先控风险，最小改法就是先别在这里新增 hard error，保持旧行为不退化；真正要和 upstream 对齐到位，则需要后续把 ChannelsLast/ChannelsLast3d 的 restride 也补上，复杂性会比这次拆分略高一些。

"resize_ only supports contiguous memory format, but got ",
static_cast<int>(*memory_format));
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

The memory_format rejection message reports static_cast<int>(*memory_format), which is hard to interpret and inconsistent with other compat ops that report unsupported MemoryFormat values more directly. Prefer emitting a clearer value (e.g., the enum name) and/or reusing the same phrasing used in other ATen compat ops (like empty/empty_like) so downstream users get consistent diagnostics.

Suggested change
static_cast<int>(*memory_format));
*memory_format);

Copilot uses AI. Check for mistakes.
}
Copy link
Copy Markdown
Contributor

@ShigureNyako ShigureNyako Apr 2, 2026

Choose a reason for hiding this comment

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

如果这次拆分 PR 先以“不要引入新的兼容回退”为目标，这里建议先不要把 memory_format 从“原来基本忽略”升级成 hard error。

具体风险在于：旧实现这里直接 reshape，memory_format 参数虽然没真正生效，但同-numel 场景至少不会在 compat 层同步报错；这版则会把 channels_last/channels_last_3d 调用直接变成异常。完整补齐 upstream 语义需要额外的 restride 支持，复杂性比这个 split PR 大一些；如果想先控风险，最小改法可以是先删掉这个 guard，后续再单独补真正的 memory-format 对齐。

Suggested change
if (memory_format.has_value()) {
TORCH_CHECK(*memory_format == at::MemoryFormat::Contiguous,
"resize_ only supports contiguous memory format, but got ",
static_cast<int>(*memory_format));
}
// Keep old compat behavior for memory_format in this split PR.
// TODO: add real ChannelsLast/ChannelsLast3d restride support in a follow-up.


std::vector<int64_t> dims(size.begin(), size.end());
int64_t new_numel = 1;
for (auto dim : dims) {
new_numel *= dim;
}
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

new_numel is computed by multiplying dims without any checks for negative dimensions or int64 overflow. If a caller passes an invalid/very large size, new_numel can wrap and the code may take the reshape path or pass sizes into set_ in a surprising way. Consider validating dim >= 0 and using overflow-safe multiplication (or an existing checked helper) before comparing against tensor_.numel().

Copilot uses AI. Check for mistakes.

if (tensor_.numel() == new_numel) {
const_cast<Tensor*>(this)->tensor_ =
paddle::experimental::reshape(tensor_, phi::IntArray(dims));
return *this;
}

auto source = tensor_.copy_to(tensor_.place(), /*blocking=*/true);
paddle::experimental::set_(const_cast<Tensor*>(this)->tensor_, source, dims);
Copy link
Copy Markdown
Contributor

@ShigureNyako ShigureNyako Apr 2, 2026

Choose a reason for hiding this comment

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

这里有一个和 upstream torch.resize_ 语义不一致的点，建议在合入前修正。

当前实现一旦 numel 变化，就会先 copy_to 再 set_，这会无条件换掉底层 storage。仓内 as_strided_scatter 也是用 copy_to(..., true) 来显式做“independent copy”，所以这里不是单纯改 shape，而是把 storage 换成了一份新副本。

但 PyTorch 的 resize_ 在 shrink 场景下不会改 underlying storage，只会在容量不够时扩容；因此 shrink -> grow back 时，旧 capacity 内被隐藏的 tail 数据应该还能重新可见。按这里的实现，tail 数据会在 copy_to 时直接丢掉，行为会变成“只保留当前可见前缀”。

建议至少补一个 round-trip 用例来锁定这点，比如 24 -> 20 -> 24 后检查 20..23 是否仍然可见；实现上也尽量改成“仅在需要扩容时变更 storage”，这样才能和 upstream 对齐。

Copy link
Copy Markdown
Member

@SigureMo SigureMo Apr 2, 2026

Choose a reason for hiding this comment

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

@ShigureNyako 给点详细的 suggestion 吧，通过 code suggestion 提出来，这里具体在兼容性的风险是什么？是否会增加很多复杂性？如果在可控范围内可以按照你说的改

Copy link
Copy Markdown
Contributor

@ShigureNyako ShigureNyako Apr 2, 2026

Choose a reason for hiding this comment

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

@ShigureNyako 给点详细的 suggestion 吧，通过 code suggestion 提出来，这里具体在兼容性的风险是什么？是否会增加很多复杂性？如果在可控范围内可以按照你说的改

收到，我刚补了两个更具体的 inline suggestion。

这里这个 storage 语义的风险点是：当前实现对所有 numel 变化都先 copy_to，这一步会把底层 storage 换成一份 independent copy。这样在 shrink -> grow back 的 round-trip 场景里，原来 hidden tail 那段数据会被直接截掉；而 upstream torch.resize_ 的 shrink 语义是不换 storage，只在容量不够时扩容，所以 tail 数据应当还能重新可见。

我给的最小改法是把这里改成直接复用当前 tensor 自己做 source，也就是和 Paddle Python 侧 resize_ 的 x.set_(x, shape) 保持一致：这样复杂性比较可控，不需要先引入完整的新 storage 管理逻辑。除此之外，再补一个 24 -> 20 -> 24 的 round-trip 用例，基本就能把这个兼容风险锁住。

Copy link
Copy Markdown
Contributor

@ShigureNyako ShigureNyako Apr 2, 2026

Choose a reason for hiding this comment

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

这里可以给一个更具体的最小改法：不要先 copy_to 再 set_，而是直接把当前 tensor 自己作为 set_ 的 source。

Paddle Python 侧 resize_ 本身就是 x.set_(x, shape)；按这个思路改，至少不会先做一次 independent copy，语义上也更接近 upstream resize_ 的“shrink 不换 storage，只在需要时扩容”。另外建议再补一个 24 -> 20 -> 24 的 round-trip 测试，锁定 tail 数据是否还能重新可见。

Suggested change
auto source = tensor_.copy_to(tensor_.place(), /*blocking=*/true);
paddle::experimental::set_(const_cast<Tensor*>(this)->tensor_, source, dims);
paddle::experimental::set_(const_cast<Tensor*>(this)->tensor_, tensor_, dims);

return *this;
}

Expand Down
32 changes: 30 additions & 2 deletions test/cpp/compat/ATen_resize_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@
#include "torch/all.h"

// ======================== resize_ tests ========================
// Note: Paddle's resize_ is implemented via reshape, which requires
// total element count to remain unchanged.
// Note: compat resize_ uses reshape when numel is unchanged, and falls back to
// set_ for storage-changing cases so repeated resize_ calls remain stable.

TEST(TensorResizeTest, ResizeBasic) {
// Create a 2x3 tensor
Expand Down Expand Up @@ -109,6 +109,34 @@ TEST(TensorResizeTest, ResizePreservesData) {
ASSERT_FLOAT_EQ(data[5], 5.0f);
}

TEST(TensorResizeTest, ResizeShrinkDifferentNumel) {
at::Tensor t = at::arange(24, at::kFloat).reshape({2, 3, 4});

t.resize_({4, 5});

Comment on lines +116 to +120
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

New behavior was added for the memory_format argument (only contiguous is accepted), but the test suite here doesn't cover that path. Adding a small gtest that passes a non-contiguous memory format and asserts the expected failure would lock in the intended compatibility behavior.

Copilot uses AI. Check for mistakes.
ASSERT_EQ(t.sizes()[0], 4);
ASSERT_EQ(t.sizes()[1], 5);

float* data = t.data_ptr<float>();
for (int i = 0; i < 20; ++i) {
ASSERT_FLOAT_EQ(data[i], static_cast<float>(i));
}
}

TEST(TensorResizeTest, ResizeGrowDifferentNumelPreservesPrefix) {
at::Tensor t = at::arange(6, at::kFloat).reshape({2, 3});

t.resize_({2, 5});

ASSERT_EQ(t.sizes()[0], 2);
ASSERT_EQ(t.sizes()[1], 5);

float* data = t.data_ptr<float>();
for (int i = 0; i < 6; ++i) {
ASSERT_FLOAT_EQ(data[i], static_cast<float>(i));
}
}

TEST(TensorResizeTest, ResizeReturnReference) {
// Create a tensor
at::Tensor t = at::zeros({2, 3});
Expand Down
Loading