diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 88f55ef73cf..ed1e2b30323 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -215,6 +215,14 @@ must work with threading** ## Testing +### Running Tests Locally + +CI is run automatically on all pull requests. However, if you want to run tests locally, here are some example commands (not exhaustive): + +- The `sh test/build_size_test.sh` script will compile the C++runtime along with portable kernels. +- The `test/run_oss_cpp_tests.sh` script will build and run C++ tests locally +- Running `pytest` from the root directory will run Python tests locally. + ### Writing Tests To help keep code quality high, ExecuTorch uses a combination of unit tests and end-to-end (e2e) tests. If you add a new feature or fix a bug, please add tests @@ -229,8 +237,6 @@ If it's not clear how to add a test for your PR, take a look at the blame for the code you're modifying and find an author who has more context. Ask them for their help in the PR comments. -The `test/run_oss_cpp_tests.sh` script will build and run C++ tests locally. - ### Continuous Integration See https://hud.pytorch.org/hud/pytorch/executorch/main for the current state of the CI (continuous integration) jobs. If `main` is broken, consider rebasing diff --git a/kernels/portable/cpu/op_full.cpp b/kernels/portable/cpu/op_full.cpp index cf73447c1bd..668033a44af 100644 --- a/kernels/portable/cpu/op_full.cpp +++ b/kernels/portable/cpu/op_full.cpp @@ -38,7 +38,8 @@ Tensor& full_out( ET_SWITCH_SCALAR_OBJ_TYPES(val_type, ctx, name, CTYPE_VAL, [&] { CTYPE_VAL val; - utils::extract_scalar(fill_value, &val); + ET_KERNEL_CHECK( + ctx, utils::extract_scalar(fill_value, &val), InvalidArgument, ); ET_SWITCH_REALHBBF16_TYPES(out_type, ctx, name, CTYPE_OUT, [&] { CTYPE_OUT val_casted = static_cast(val); diff --git a/kernels/portable/cpu/op_full_like.cpp b/kernels/portable/cpu/op_full_like.cpp index 508fa512197..2aeb45d22f4 100644 --- a/kernels/portable/cpu/op_full_like.cpp +++ b/kernels/portable/cpu/op_full_like.cpp @@ -54,7 +54,8 @@ Tensor& full_like_out( ET_SWITCH_REALB_TYPES(val_type, ctx, name, CTYPE_VAL, [&] { CTYPE_VAL val; - utils::extract_scalar(fill_value, &val); + ET_KERNEL_CHECK( + ctx, utils::extract_scalar(fill_value, &val), InvalidArgument, ); ET_SWITCH_REALHBBF16_TYPES(out_type, ctx, name, CTYPE_OUT, [&] { CTYPE_OUT val_casted = static_cast(val); diff --git a/kernels/portable/cpu/op_scatter.cpp b/kernels/portable/cpu/op_scatter.cpp index 4fcf05e49c8..af4ca8a8390 100644 --- a/kernels/portable/cpu/op_scatter.cpp +++ b/kernels/portable/cpu/op_scatter.cpp @@ -156,7 +156,7 @@ Tensor& scatter_value_out( ET_SWITCH_SCALAR_OBJ_TYPES(val_type, ctx, name, CTYPE_VAL, [&] { CTYPE_VAL val; - utils::extract_scalar(value, &val); + ET_KERNEL_CHECK(ctx, utils::extract_scalar(value, &val), InvalidArgument, ); ET_SWITCH_REALHBBF16_TYPES(in.scalar_type(), ctx, name, CTYPE, [&]() { scatter_value_helper(in, dim, index, val, out); diff --git a/kernels/portable/cpu/util/broadcast_util.cpp b/kernels/portable/cpu/util/broadcast_util.cpp index dc74ca5ec3e..90f84854e80 100644 --- a/kernels/portable/cpu/util/broadcast_util.cpp +++ b/kernels/portable/cpu/util/broadcast_util.cpp @@ -215,18 +215,16 @@ ET_NODISCARD Error get_broadcast_target_size( size_t* out_dim) { if ET_UNLIKELY (!tensors_are_broadcastable_between(a_size, b_size)) { #ifdef ET_LOG_ENABLED - const auto a_shape_str = tensor_shape_to_c_string( - executorch::runtime::Span( - a_size.data(), a_size.size())); - const auto b_shape_str = tensor_shape_to_c_string( - executorch::runtime::Span( - b_size.data(), b_size.size())); -#endif + executorch::runtime::Span a_size_span( + a_size.data(), a_size.size()); + executorch::runtime::Span b_size_span( + b_size.data(), b_size.size()); ET_LOG( Error, "Two input tensors should be broadcastable but got shapes %s and %s.", - a_shape_str.data(), - b_shape_str.data()); + tensor_shape_to_c_string(a_size_span).data(), + tensor_shape_to_c_string(b_size_span).data()); +#endif return executorch::runtime::Error::InvalidArgument; } diff --git a/runtime/core/portable_type/tensor_impl.cpp b/runtime/core/portable_type/tensor_impl.cpp index 05e65375b66..5eecf591cff 100644 --- a/runtime/core/portable_type/tensor_impl.cpp +++ b/runtime/core/portable_type/tensor_impl.cpp @@ -96,20 +96,18 @@ Error TensorImpl::internal_resize_contiguous(ArrayRef new_sizes) { case TensorShapeDynamism::STATIC: if (!std::equal(sizes_, sizes_ + dim_, new_sizes.begin())) { #ifdef ET_LOG_ENABLED - auto old_sizes_str = executorch::runtime::tensor_shape_to_c_string( - executorch::runtime::Span( - sizes().data(), sizes().size())); - auto new_sizes_str = executorch::runtime::tensor_shape_to_c_string( - executorch::runtime::Span( - new_sizes.data(), new_sizes.size())); -#endif - - ET_CHECK_OR_RETURN_ERROR( - false, - NotSupported, + executorch::runtime::Span sizes_span( + sizes().data(), sizes().size()); + executorch::runtime::Span new_sizes_span( + new_sizes.data(), new_sizes.size()); + ET_LOG( + Error, "Attempted to resize a static tensor. Expected shape %s, but received %s.", - old_sizes_str.data(), - new_sizes_str.data()) + executorch::runtime::tensor_shape_to_c_string(sizes_span).data(), + executorch::runtime::tensor_shape_to_c_string(new_sizes_span) + .data()); +#endif + return executorch::runtime::Error::NotSupported; } break; diff --git a/test/build_size_test.sh b/test/build_size_test.sh index 732880c60ac..823b399fe34 100644 --- a/test/build_size_test.sh +++ b/test/build_size_test.sh @@ -11,11 +11,15 @@ set -e # shellcheck source=/dev/null source "$(dirname "${BASH_SOURCE[0]}")/../.ci/scripts/utils.sh" +# TODO(#8149): Remove -Wno-sign-compare +# TODO(#8357): Remove -Wno-int-in-bool-context +COMMON_CXXFLAGS="-fno-exceptions -fno-rtti -Wall -Werror -Wno-sign-compare -Wno-unknown-pragmas -Wno-int-in-bool-context" + cmake_install_executorch_lib() { echo "Installing libexecutorch.a" clean_executorch_install_folders - CXXFLAGS="-fno-exceptions -fno-rtti" retry cmake -DBUCK2="$BUCK2" \ + CXXFLAGS="$COMMON_CXXFLAGS" retry cmake -DBUCK2="$BUCK2" \ -DCMAKE_CXX_STANDARD_REQUIRED=ON \ -DCMAKE_INSTALL_PREFIX=cmake-out \ -DCMAKE_BUILD_TYPE=Release \ @@ -27,7 +31,7 @@ cmake_install_executorch_lib() { } test_cmake_size_test() { - CXXFLAGS="-fno-exceptions -fno-rtti" retry cmake -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=cmake-out -Bcmake-out/test test + CXXFLAGS="$COMMON_CXXFLAGS" retry cmake -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=cmake-out -Bcmake-out/test test echo "Build size test" cmake --build cmake-out/test -j9 --config Release