Skip to content

Support bool type for Pad CPU #7856

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
faxu merged 3 commits into from
May 27, 2021
Merged

Support bool type for Pad CPU #7856

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8e6e145
Initial commit
hariharans29 May 27, 2021
e3e2502
update
hariharans29 May 27, 2021
02f6ac3
nit
hariharans29 May 27, 2021
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
33 changes: 28 additions & 5 deletions onnxruntime/core/providers/cpu/tensor/pad.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,22 @@ ORT_SPECIFY_OP_KERNEL_ARG_DEFAULT_TYPES(
int8_t,
uint8_t);

ORT_SPECIFY_OP_KERNEL_ARG_DEFAULT_TYPES(
kCpuExecutionProvider, kOnnxDomain, Pad, 13, Input, 0,
float,
double,
int32_t,
int64_t,
uint32_t,
uint64_t,
int8_t,
uint8_t,
bool);

ORT_SPECIFY_OP_KERNEL_ARG_REQUIRED_TYPES(
kCpuExecutionProvider, kOnnxDomain, Pad, 11, Input, 0, int32_t, int64_t);
ORT_SPECIFY_OP_KERNEL_ARG_REQUIRED_TYPES(
kCpuExecutionProvider, kOnnxDomain, Pad, 13, Input, 0, int32_t, int64_t);
} // namespace op_kernel_type_control

using Pad2Types = ORT_OP_KERNEL_ARG_DEFAULT_TYPE_LIST(
Expand All @@ -66,11 +80,16 @@ using Pad11Types = ORT_OP_KERNEL_ARG_DEFAULT_TYPE_LIST(
kCpuExecutionProvider, kOnnxDomain, Pad, 11, Input, 0);
using EnabledPad11Types = ORT_OP_KERNEL_ARG_ENABLED_TYPE_LIST(
kCpuExecutionProvider, kOnnxDomain, Pad, 11, Input, 0);
using Pad13Types = ORT_OP_KERNEL_ARG_DEFAULT_TYPE_LIST(
kCpuExecutionProvider, kOnnxDomain, Pad, 13, Input, 0);
using EnabledPad13Types = ORT_OP_KERNEL_ARG_ENABLED_TYPE_LIST(
kCpuExecutionProvider, kOnnxDomain, Pad, 13, Input, 0);

using AllEnabledPadTypes =
utils::TypeSetUnion<
EnabledPad2Types,
EnabledPad11Types>;
EnabledPad11Types,
EnabledPad13Types>;

// only float type is supported for opset-10
ONNX_CPU_OPERATOR_VERSIONED_KERNEL(
Expand Down Expand Up @@ -98,10 +117,14 @@ ONNX_CPU_OPERATOR_VERSIONED_KERNEL(
ONNX_CPU_OPERATOR_KERNEL(
Pad,
13,
KernelDefBuilder().TypeConstraint(
"T",
BuildKernelDefConstraintsFromTypeList<Pad11Types>(),
BuildKernelDefConstraintsFromTypeList<EnabledPad11Types>()),
KernelDefBuilder()
.TypeConstraint(
"T",
BuildKernelDefConstraintsFromTypeList<Pad13Types>(),
BuildKernelDefConstraintsFromTypeList<EnabledPad13Types>())
.FixedTypeConstraintForHash(
"T",
BuildKernelDefConstraintsFromTypeList<Pad11Types>()),
Pad);

Comment thread
hariharans29 marked this conversation as resolved.
// This is the general padding method to n-dimensionally do edge or reflection padding (based on the inputDelta values)
Expand Down
68 changes: 52 additions & 16 deletions onnxruntime/test/providers/cpu/tensor/pad_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,15 @@ static void RunOnnxOpsetTypedTest(
if (opset >= 11) {
test.AddInput<int64_t>("pads", {static_cast<int64_t>(pads.size())}, pads);
test.AddInput<T>("value", {1}, {value});
}
else {
} else {
test.AddAttribute("pads", pads);
test.AddAttribute("value", static_cast<float>(value));
}
test.AddOutput<T>("output", output_dims, output);
if (opset >= 11) {
// TensorRT do not yet support opset-11 and builds break on this test, hence exclude the EP
test.Run(expect, error_msg, {kTensorrtExecutionProvider});
}
else {
} else {
#if defined(OPENVINO_CONFIG_MYRIAD) || defined(OPENVINO_CONFIG_VAD_M)
test.Run(expect, error_msg, {kOpenVINOExecutionProvider});
#else
Expand All @@ -56,16 +54,24 @@ static void RunAllOpsetAllDomainPadTests(
std::string mode = "constant",
OpTester::ExpectResult expect = OpTester::ExpectResult::kExpectSuccess,
const std::string& error_msg = "") {
// ONNX domain opset-11 is the only one to support all data types
// Test opset-11 and opset-13 kernels of Pad
RunOnnxOpsetTypedTest<T, 11>(input_dims,
input,
pads,
value,
output_dims,
output,
mode, expect, error_msg);

RunOnnxOpsetTypedTest<T, 13>(input_dims,
input,
pads,
value,
output_dims,
output,
mode, expect, error_msg);
}
template<>
template <>
void RunAllOpsetAllDomainPadTests<>(
const std::vector<int64_t>& input_dims,
const std::vector<double>& input,
Expand All @@ -76,24 +82,34 @@ void RunAllOpsetAllDomainPadTests<>(
std::string mode,
OpTester::ExpectResult expect,
const std::string& error_msg) {
// ONNX domain supports double type
// Test opset-10, opset-11 and opset-13 kernels of Pad (for double type)
RunOnnxOpsetTypedTest<double, 10>(input_dims,
input,
pads,
value,
output_dims,
output,
mode, expect, error_msg);

RunOnnxOpsetTypedTest<double, 11>(input_dims,
input,
pads,
value,
output_dims,
output,
mode, expect, error_msg);

RunOnnxOpsetTypedTest<double, 13>(input_dims,
input,
pads,
value,
output_dims,
output,
mode, expect, error_msg);
}

// There is only support for float type for MSDomain kernel in ORT
template<>
template <>
void RunAllOpsetAllDomainPadTests<>(
const std::vector<int64_t>& input_dims,
const std::vector<float>& input,
Expand All @@ -104,13 +120,15 @@ void RunAllOpsetAllDomainPadTests<>(
std::string mode,
OpTester::ExpectResult expect,
const std::string& error_msg) {
// Test opset-10, opset-11 and opset-13 kernels of Pad (for float type)
RunOnnxOpsetTypedTest<float, 10>(input_dims,
input,
pads,
value,
output_dims,
output,
mode, expect, error_msg);

RunOnnxOpsetTypedTest<float, 11>(input_dims,
input,
pads,
Expand All @@ -119,6 +137,14 @@ void RunAllOpsetAllDomainPadTests<>(
output,
mode, expect, error_msg);

RunOnnxOpsetTypedTest<float, 13>(input_dims,
input,
pads,
value,
output_dims,
output,
mode, expect, error_msg);

#ifndef DISABLE_CONTRIB_OPS

// MSFT domain opset-1 (contrib op)
Expand All @@ -129,7 +155,7 @@ void RunAllOpsetAllDomainPadTests<>(
test3.AddInput<float>("value", {1}, {value});
test3.AddOutput<float>("output", output_dims, output);
//TensorRT does not support pads as an input
test3.Run(expect, error_msg, {kTensorrtExecutionProvider,kOpenVINOExecutionProvider});
test3.Run(expect, error_msg, {kTensorrtExecutionProvider, kOpenVINOExecutionProvider});

#endif
}
Expand Down Expand Up @@ -679,19 +705,19 @@ TYPED_TEST(PadOpTest, Pad_Constant_DimWithZeroInput) {
{T(1), T(1), T(1), T(1), T(1), T(1), T(1), T(1)});
}
// Added output shape verification b/w the output shape generated by operator specific ONNX inference and
// the output shape generated by operator specific ORT implementation. After adding this verification,
// the output shape generated by operator specific ORT implementation. After adding this verification,
// this test logs warning as validation fails for 2 data types out of 8 data types i.e. Float and Double.
// Reason:
// Pad ORT implementation output shape does not match with Pad ONNX inference function output shape.
//
// For Float and Double this test gets executed for 2 different opset version, 10 and 11. Specifically this
// test is failing for opset version 10.
// Investigation Analysis: Different ONNX inference class/method gets executed per opset version. Main difference b/w the 2
//
// For Float and Double this test gets executed for 2 different opset version, 10 and 11. Specifically this
// test is failing for opset version 10.
// Investigation Analysis: Different ONNX inference class/method gets executed per opset version. Main difference b/w the 2
// pad operator ONNX inference class/method is:
// Older Pad operator ONNX inference: Accepts "pads and values" as attribute.
// Newer Pad operator ONNX inference: Accetps "pads and values" as input.
// For newer version, "pads & values" fields have not been added as initializer, thus instead of shape
// inference, rank inference gets triggered. Whereas, in older version shape inference gets executed
// For newer version, "pads & values" fields have not been added as initializer, thus instead of shape
// inference, rank inference gets triggered. Whereas, in older version shape inference gets executed
// as "pads & values" fields have been added as attribute.
// In order to remove the warning, shape inference methods needs to be fixed.

Expand Down Expand Up @@ -743,5 +769,15 @@ TYPED_TEST(PadOpTest, Pad_Reflect_DimWithZeroInput) {
"Cannot use 'reflect' mode to pad dimension with a value of 0. Input shape:{0,2,1}");
}

TEST(PadOpTest, BoolType) {
OpTester test("Pad", 13);
test.AddAttribute("mode", "constant");
test.AddInput<bool>("data", {3, 2}, {true, false, true, false, true, false});
test.AddInput<int64_t>("pads", {4}, {0, 2, 0, 0});

@skottmckay skottmckay May 27, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: easier to read if this is 3 rows of 2 values and the expected output is 3 rows of 4 values.

test.AddInput<bool>("value", {1}, {true});
test.AddOutput<bool>("output", {3, 4}, {true, true, true, false, true, true, true, false, true, true, true, false});
test.Run(OpTester::ExpectResult::kExpectSuccess, "", {kTensorrtExecutionProvider});
}

} // namespace test
} // namespace onnxruntime