Skip to content

Use non-fatal input checks for view operator #9264

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
merged 5 commits into from
Mar 15, 2025
Merged

Use non-fatal input checks for view operator #9264

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6505ac9
Use non-fatal input checks for view operator
zhenyan-zhang-meta Mar 14, 2025
3074e44
Merge branch 'main' into issue/9130
zhenyan-zhang-meta Mar 14, 2025
980d525
Update exception type for set tensor data to Internal
zhenyan-zhang-meta Mar 14, 2025
5342a40
Merge branch 'issue/9130' of github.com:zhenyan-zhang-meta/executorch…
zhenyan-zhang-meta Mar 14, 2025
df7b286
Update tests to reflect non-fatal input check changes
zhenyan-zhang-meta Mar 14, 2025
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: 15 additions & 5 deletions kernels/prim_ops/et_view.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,28 +72,38 @@ void et_view(KernelRuntimeContext& context, EValue** stack) {
auto size = (*stack[1]).toIntList();
auto out = (*stack[2]).toTensor();

ET_CHECK(tensors_have_same_dtype(self, out));
ET_KERNEL_CHECK(
context, tensors_have_same_dtype(self, out), InvalidArgument, );
Copy link
Contributor

@Gasoonjia Gasoonjia Mar 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

plz remove the extra , on the end of function; same as below

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The commas has to be there since it needs an extra, though empty, param passed in for retval (return value).

Similar commas can be seen in main branch:

executorch/kernels/optimized/cpu/op_add.cpp

Lines 47 to 50 in bb3b623

ET_KERNEL_CHECK(
ctx,
torch::executor::native::utils::extract_scalar(alpha, &alpha_val),
InvalidArgument, );

Copy link
Contributor

Choose a reason for hiding this comment

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

oh yeah you're right i forgot that. Thanks for noticing!


// Compute output size
SizesType expected_output_size[kTensorDimensionLimit];
ET_CHECK(get_view_target_size(self, size, out.dim(), expected_output_size));
ET_KERNEL_CHECK(
context,
get_view_target_size(self, size, out.dim(), expected_output_size),
InvalidArgument, );

// Resize for dynamic shape
ET_CHECK_MSG(
ET_KERNEL_CHECK_MSG(
context,
resize_tensor(
out, {expected_output_size, static_cast<size_t>(out.dim())}) ==
Error::Ok,
Internal,
,
"Failed to resize output tensor.");

// Do some checks
ET_CHECK(self.numel() == out.numel());
ET_KERNEL_CHECK(context, self.numel() == out.numel(), InvalidArgument, );

// Update data ptr
ET_CHECK_MSG(
ET_KERNEL_CHECK_MSG(
context,
internal::set_tensor_data(
out,
/*buffer=*/self.mutable_data_ptr(),
/*buffer_size=*/out.nbytes()) == Error::Ok,
Internal,
,
"Failed to set data_ptr for out to self.");
}

Expand Down
1 change: 1 addition & 0 deletions kernels/prim_ops/test/TARGETS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ runtime.cxx_test(
],
deps = [
"//executorch/kernels/prim_ops:prim_ops_registry", # @manual
"//executorch/kernels/test:test_util", # @manual
"//executorch/runtime/core:evalue", # @manual
"//executorch/runtime/core/exec_aten:lib", # @manual
"//executorch/runtime/core/exec_aten/testing_util:tensor_util", # @manual
Expand Down
79 changes: 39 additions & 40 deletions kernels/prim_ops/test/prim_ops_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

#include <gtest/gtest.h>

#include <executorch/kernels/test/TestUtil.h>
#include <executorch/runtime/core/evalue.h>
#include <executorch/runtime/core/exec_aten/exec_aten.h>
#include <executorch/runtime/core/exec_aten/testing_util/tensor_factory.h>
Expand All @@ -16,7 +17,6 @@
#include <executorch/runtime/kernel/kernel_runtime_context.h>
#include <executorch/runtime/kernel/operator_registry.h>
#include <executorch/runtime/platform/runtime.h>
#include <executorch/test/utils/DeathTest.h>
#include <cstdint>
#include <cstdio>

Expand All @@ -27,12 +27,10 @@ using torch::executor::resize_tensor;
namespace torch {
namespace executor {

class RegisterPrimOpsTest : public ::testing::Test {
class RegisterPrimOpsTest : public OperatorTest {
protected:
KernelRuntimeContext context;
void SetUp() override {
torch::executor::runtime_init();
context = KernelRuntimeContext();
context_ = KernelRuntimeContext();
}
};

Expand All @@ -57,7 +55,7 @@ TEST_F(RegisterPrimOpsTest, SymSizeReturnsCorrectValue) {
stack[i] = &values[i];
}

getOpsFn("aten::sym_size.int")(context, stack);
getOpsFn("aten::sym_size.int")(context_, stack);

int64_t expected = 5;
EXPECT_EQ(stack[2]->toInt(), expected);
Expand All @@ -77,7 +75,7 @@ TEST_F(RegisterPrimOpsTest, SymNumelReturnsCorrectValue) {
stack[i] = &values[i];
}

getOpsFn("aten::sym_numel")(context, stack);
getOpsFn("aten::sym_numel")(context_, stack);

int64_t expected = 15;
EXPECT_EQ(stack[1]->toInt(), expected);
Expand All @@ -97,28 +95,28 @@ TEST_F(RegisterPrimOpsTest, TestAlgebraOps) {
stack[i] = &values[i];
}

getOpsFn("executorch_prim::add.Scalar")(context, stack);
getOpsFn("executorch_prim::add.Scalar")(context_, stack);
EXPECT_EQ(stack[2]->toInt(), 7);

getOpsFn("executorch_prim::sub.Scalar")(context, stack);
getOpsFn("executorch_prim::sub.Scalar")(context_, stack);
EXPECT_EQ(stack[2]->toInt(), -1);

getOpsFn("executorch_prim::mul.Scalar")(context, stack);
getOpsFn("executorch_prim::mul.Scalar")(context_, stack);
EXPECT_EQ(stack[2]->toInt(), 12);

getOpsFn("executorch_prim::floordiv.Scalar")(context, stack);
getOpsFn("executorch_prim::floordiv.Scalar")(context_, stack);
EXPECT_EQ(stack[2]->toInt(), 0);

getOpsFn("executorch_prim::truediv.Scalar")(context, stack);
getOpsFn("executorch_prim::truediv.Scalar")(context_, stack);
EXPECT_FLOAT_EQ(stack[2]->toDouble(), 0.75);

getOpsFn("executorch_prim::mod.int")(context, stack);
getOpsFn("executorch_prim::mod.int")(context_, stack);
EXPECT_EQ(stack[2]->toInt(), 3);

getOpsFn("executorch_prim::mod.Scalar")(context, stack);
getOpsFn("executorch_prim::mod.Scalar")(context_, stack);
EXPECT_EQ(stack[2]->toInt(), 3);

getOpsFn("executorch_prim::sym_float.Scalar")(context, stack);
getOpsFn("executorch_prim::sym_float.Scalar")(context_, stack);
EXPECT_FLOAT_EQ(stack[1]->toDouble(), 3.0);
}

Expand Down Expand Up @@ -155,7 +153,7 @@ TEST_F(RegisterPrimOpsTest, TestETCopyIndex) {
stack[2] = &values[2];

// Simple test to copy to index 0.
getOpsFn("executorch_prim::et_copy_index.tensor")(context, stack);
getOpsFn("executorch_prim::et_copy_index.tensor")(context_, stack);

EXPECT_EQ(copy_to.sizes()[0], 1);
EXPECT_EQ(copy_to.sizes()[1], 2);
Expand All @@ -164,7 +162,7 @@ TEST_F(RegisterPrimOpsTest, TestETCopyIndex) {
values[1] = tf.make({2}, {5, 6});
values[2] = EValue((int64_t)1);
// Copy to the next index, 1.
getOpsFn("executorch_prim::et_copy_index.tensor")(context, stack);
getOpsFn("executorch_prim::et_copy_index.tensor")(context_, stack);

EXPECT_EQ(copy_to.sizes()[0], 2);
EXPECT_EQ(copy_to.sizes()[1], 2);
Expand Down Expand Up @@ -193,7 +191,7 @@ TEST_F(RegisterPrimOpsTest, TestETCopyIndexMismatchShape) {
// copy_to.sizes[1:] and to_copy.sizes[:] don't match each other
// which is a pre-requisite for this operator.
ET_EXPECT_DEATH(
getOpsFn("executorch_prim::et_copy_index.tensor")(context, stack), "");
getOpsFn("executorch_prim::et_copy_index.tensor")(context_, stack), "");
}

TEST_F(RegisterPrimOpsTest, TestETCopyIndexStaticShape) {
Expand All @@ -217,7 +215,7 @@ TEST_F(RegisterPrimOpsTest, TestETCopyIndexStaticShape) {
stack[2] = &values[2];

// Copy and replace at index 1.
getOpsFn("executorch_prim::et_copy_index.tensor")(context, stack);
getOpsFn("executorch_prim::et_copy_index.tensor")(context_, stack);
EXPECT_EQ(copy_to.sizes()[0], 2);
EXPECT_EQ(copy_to.sizes()[1], 2);
EXPECT_TENSOR_EQ(copy_to, tf.make({2, 2}, {1, 2, 5, 6}));
Expand All @@ -228,7 +226,7 @@ TEST_F(RegisterPrimOpsTest, TestETCopyIndexStaticShape) {
index = 2;
values[2] = EValue(index);
ET_EXPECT_DEATH(
getOpsFn("executorch_prim::et_copy_index.tensor")(context, stack), "");
getOpsFn("executorch_prim::et_copy_index.tensor")(context_, stack), "");
#endif
}

Expand All @@ -246,19 +244,19 @@ TEST_F(RegisterPrimOpsTest, TestBooleanOps) {
stack[i] = &values[i];
}

getOpsFn("executorch_prim::ge.Scalar")(context, stack);
getOpsFn("executorch_prim::ge.Scalar")(context_, stack);
EXPECT_EQ(stack[2]->toBool(), false);

getOpsFn("executorch_prim::gt.Scalar")(context, stack);
getOpsFn("executorch_prim::gt.Scalar")(context_, stack);
EXPECT_EQ(stack[2]->toBool(), false);

getOpsFn("executorch_prim::le.Scalar")(context, stack);
getOpsFn("executorch_prim::le.Scalar")(context_, stack);
EXPECT_EQ(stack[2]->toBool(), true);

getOpsFn("executorch_prim::lt.Scalar")(context, stack);
getOpsFn("executorch_prim::lt.Scalar")(context_, stack);
EXPECT_EQ(stack[2]->toBool(), true);

getOpsFn("executorch_prim::eq.Scalar")(context, stack);
getOpsFn("executorch_prim::eq.Scalar")(context_, stack);
EXPECT_EQ(stack[2]->toBool(), false);
}

Expand All @@ -277,7 +275,7 @@ TEST_F(RegisterPrimOpsTest, LocalScalarDenseReturnsCorrectValue) {
stack[i] = &values[i];
}

getOpsFn("aten::_local_scalar_dense")(context, stack);
getOpsFn("aten::_local_scalar_dense")(context_, stack);

int64_t expected = 1;
EXPECT_EQ(stack[1]->toInt(), expected);
Expand All @@ -295,7 +293,7 @@ TEST_F(RegisterPrimOpsTest, NegScalarReturnsCorrectValue) {
stack[i] = &values[i];
}

getOpsFn("executorch_prim::neg.Scalar")(context, stack);
getOpsFn("executorch_prim::neg.Scalar")(context_, stack);

EXPECT_EQ(stack[1]->toDouble(), -5.0f);

Expand All @@ -305,7 +303,7 @@ TEST_F(RegisterPrimOpsTest, NegScalarReturnsCorrectValue) {
values[0] = EValue(a);
values[1] = EValue(b);

getOpsFn("executorch_prim::neg.Scalar")(context, stack);
getOpsFn("executorch_prim::neg.Scalar")(context_, stack);

EXPECT_EQ(stack[1]->toInt(), -5l);
}
Expand All @@ -327,7 +325,7 @@ TEST_F(RegisterPrimOpsTest, TestNegScalarWithTensorDies) {
}

// Try to negate a tensor, which should cause a runtime error.
ET_EXPECT_DEATH(getOpsFn("executorch_prim::neg.Scalar")(context, stack), "");
ET_EXPECT_DEATH(getOpsFn("executorch_prim::neg.Scalar")(context_, stack), "");
}

TEST_F(RegisterPrimOpsTest, TestETView) {
Expand Down Expand Up @@ -410,9 +408,9 @@ TEST_F(RegisterPrimOpsTest, TestETView) {

// Bad stacks expect death
for (int i = 0; i < N_BAD_STACKS; i++) {
ET_EXPECT_DEATH(
getOpsFn("executorch_prim::et_view.default")(context, bad_stacks[i]),
"");
ET_EXPECT_KERNEL_FAILURE(
context_,
getOpsFn("executorch_prim::et_view.default")(context_, bad_stacks[i]));
}

constexpr int N_GOOD_STACKS = N_GOOD_OUTS;
Expand All @@ -422,7 +420,7 @@ TEST_F(RegisterPrimOpsTest, TestETView) {

// Good outs expect no death and correct output
for (int i = 0; i < N_GOOD_STACKS; i++) {
getOpsFn("executorch_prim::et_view.default")(context, good_out_stacks[i]);
getOpsFn("executorch_prim::et_view.default")(context_, good_out_stacks[i]);
EXPECT_TENSOR_EQ(good_outs[i], tf.make({1, 3, 2}, {1, 2, 3, 4, 5, 6}));
EXPECT_EQ(good_outs[i].const_data_ptr(), self.const_data_ptr());
}
Expand Down Expand Up @@ -456,7 +454,7 @@ TEST_F(RegisterPrimOpsTest, TestETViewDynamic) {

EValue* stack[3] = {&self_evalue, &size_int_list_evalue, &out_evalue};

getOpsFn("executorch_prim::et_view.default")(context, stack);
getOpsFn("executorch_prim::et_view.default")(context_, stack);

EXPECT_TENSOR_EQ(out, tf.make({1, 3, 1}, {1, 2, 3}));
EXPECT_EQ(out.const_data_ptr(), self.const_data_ptr());
Expand Down Expand Up @@ -493,14 +491,15 @@ TEST_F(RegisterPrimOpsTest, TestETViewEmpty) {

// good size test
EValue* stack[3] = {&self_evalue, &size_int_list_evalue, &out_evalue};
getOpsFn("executorch_prim::et_view.default")(context, stack);
getOpsFn("executorch_prim::et_view.default")(context_, stack);
EXPECT_TENSOR_EQ(out, tf.make({3, 1, 0}, {}));
EXPECT_EQ(out.const_data_ptr(), self.const_data_ptr());

// bad size test
EValue* bad_stack[3] = {&self_evalue, &bad_size_int_list_evalue, &out_evalue};
ET_EXPECT_DEATH(
getOpsFn("executorch_prim::et_view.default")(context, bad_stack), "");
ET_EXPECT_KERNEL_FAILURE(
context_,
getOpsFn("executorch_prim::et_view.default")(context_, bad_stack));
}

TEST_F(RegisterPrimOpsTest, TestCeil) {
Expand All @@ -518,7 +517,7 @@ TEST_F(RegisterPrimOpsTest, TestCeil) {
stack[j] = &values[j];
}

getOpsFn("executorch_prim::ceil.Scalar")(context, stack);
getOpsFn("executorch_prim::ceil.Scalar")(context_, stack);
EXPECT_EQ(stack[1]->toInt(), expected[i]);
}
}
Expand All @@ -539,7 +538,7 @@ TEST_F(RegisterPrimOpsTest, TestRound) {
stack[j] = &values[j];
}

getOpsFn("executorch_prim::round.Scalar")(context, stack);
getOpsFn("executorch_prim::round.Scalar")(context_, stack);
EXPECT_EQ(stack[1]->toInt(), expected[i]);
}
}
Expand All @@ -559,7 +558,7 @@ TEST_F(RegisterPrimOpsTest, TestTrunc) {
stack[j] = &values[j];
}

getOpsFn("executorch_prim::trunc.Scalar")(context, stack);
getOpsFn("executorch_prim::trunc.Scalar")(context_, stack);
EXPECT_EQ(stack[1]->toInt(), expected[i]);
}
}
Expand Down
Loading