Skip to content

[Cpp API Compatibility] add layout API #77185

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 1 commit into from
Jan 12, 2026
Merged

[Cpp API Compatibility] add layout API #77185

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
20 changes: 20 additions & 0 deletions paddle/phi/api/include/compat/ATen/core/TensorBase.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

#include <ATen/core/TensorAccessor.h>
#include <c10/core/Device.h>
#include <c10/core/Layout.h>
#include <c10/core/MemoryFormat.h>
#include <c10/core/Scalar.h>
#include <c10/core/ScalarType.h>
Expand Down Expand Up @@ -207,6 +208,25 @@ class PADDLE_API TensorBase {

bool defined() const { return tensor_.defined(); }

Layout layout() const {
switch (tensor_.layout()) {
case common::DataLayout::STRIDED:
case common::DataLayout::NCHW:
case common::DataLayout::NHWC:
case common::DataLayout::NCDHW:
case common::DataLayout::NDHWC:
return c10::kStrided;
case common::DataLayout::SPARSE_COO:
return c10::kSparse;
case common::DataLayout::SPARSE_CSR:
return c10::kSparseCsr;
case common::DataLayout::ONEDNN:
return c10::kMkldnn;
Copy link

Copilot AI Jan 4, 2026

Choose a reason for hiding this comment

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

The switch statement is missing cases for common::DataLayout::UNDEFINED and common::DataLayout::ANY. According to the paddle/common/layout.h file, these are valid DataLayout enum values (with ANY being an alias for UNDEFINED). While they fall through to the default case and return c10::kStrided, it would be clearer to explicitly handle these cases or document the intended behavior for undefined/uninitialized layouts.

Suggested change
return c10::kMkldnn;
return c10::kMkldnn;
case common::DataLayout::UNDEFINED:
case common::DataLayout::ANY:
// Treat undefined/any Paddle layouts as strided in ATen.
return c10::kStrided;

Copilot uses AI. Check for mistakes.
default:
return c10::kStrided;
}
Comment on lines +212 to +227
Copy link

Copilot AI Jan 4, 2026

Choose a reason for hiding this comment

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

The switch statement is missing a case for common::DataLayout::PSTRING_UNION. According to the paddle/common/layout.h file, PSTRING_UNION is a valid DataLayout enum value. Without an explicit case for it, it will fall through to the default case and return c10::kStrided, which may not be the correct mapping. Consider adding an explicit case for PSTRING_UNION or documenting why it should default to kStrided.

Copilot uses AI. Check for mistakes.
}
Comment on lines +211 to +228
Copy link

Copilot AI Jan 4, 2026

Choose a reason for hiding this comment

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

The layout() method lacks documentation. Consider adding a comment explaining what the method returns and how Paddle's DataLayout values map to c10::Layout values. This would help developers understand the mapping logic, especially for edge cases like PSTRING_UNION that fall through to the default case.

Copilot uses AI. Check for mistakes.

// Return a `TensorAccessor` for CPU `Tensor`s. You have to specify scalar
// type and
// dimension.
Expand Down
14 changes: 14 additions & 0 deletions test/cpp/compat/compat_basic_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include <ATen/cuda/EmptyTensor.h>
#include <ATen/native/cuda/Resize.h>
#include <ATen/ops/tensor.h>
#include <c10/core/Layout.h>
#include <c10/core/ScalarType.h>
#include <c10/core/SymInt.h>
#include <c10/core/TensorOptions.h>
Expand Down Expand Up @@ -340,3 +341,16 @@ TEST(TestTensorOperators, SubScriptOperator) {
ASSERT_EQ(tensor_2.data_ptr<float>()[i], static_cast<float>(i + offset));
}
}

TEST(TensorBaseTest, LayoutAPI) {
// Test layout() API for strided tensors
at::TensorBase tensor = at::ones({2, 3}, at::kFloat);

// Default tensor should have Strided layout
ASSERT_EQ(tensor.layout(), c10::kStrided);

// Test layout output stream operator
std::ostringstream oss;
oss << tensor.layout();
ASSERT_EQ(oss.str(), "Strided");
}
Comment on lines +345 to +356
Copy link

Copilot AI Jan 4, 2026

Choose a reason for hiding this comment

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

The test only covers the strided layout case. Consider adding test cases for other layout types that are supported by the layout() method, such as SPARSE_COO (maps to c10::kSparse), SPARSE_CSR (maps to c10::kSparseCsr), and ONEDNN (maps to c10::kMkldnn). This would ensure the switch statement logic in the layout() method is properly validated for all supported layout types.

Copilot uses AI. Check for mistakes.
Loading