From 99c47d8b5a1755d02ce0b8ce8a11d2c3ba4316d8 Mon Sep 17 00:00:00 2001 From: jcwchen Date: Fri, 24 Jun 2022 11:30:32 -0700 Subject: [PATCH 1/8] Add warning about future computation change for Convtranspose with auto_pad --- onnxruntime/core/providers/cpu/nn/conv_transpose_attributes.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/onnxruntime/core/providers/cpu/nn/conv_transpose_attributes.h b/onnxruntime/core/providers/cpu/nn/conv_transpose_attributes.h index ee74877da8d86..a1b1848ae1d08 100644 --- a/onnxruntime/core/providers/cpu/nn/conv_transpose_attributes.h +++ b/onnxruntime/core/providers/cpu/nn/conv_transpose_attributes.h @@ -176,6 +176,8 @@ struct ConvTransposeAttributes : public ConvAttributes { void DistributePadding(AutoPadType pad_type, const int64_t& total_pad, int64_t& pad_head, int64_t& pad_tail) const { + // TODO (#9740) ORT 1.13 we will correct the logic in the following lines + LOGS_DEFAULT(WARNING) << "The pad result for SAME_UPPER and SAME_LOWER will be corrected in next ORT 1.13."; if (pad_type == AutoPadType::SAME_UPPER) { // pad more on head when total_pad is odd. pad_head = total_pad - total_pad / 2; pad_tail = total_pad / 2; From 554e5bc42af9057656eac5a4c2e94754f95e6221 Mon Sep 17 00:00:00 2001 From: jcwchen Date: Fri, 24 Jun 2022 11:42:55 -0700 Subject: [PATCH 2/8] improve msg --- onnxruntime/core/providers/cpu/nn/conv_transpose_attributes.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/onnxruntime/core/providers/cpu/nn/conv_transpose_attributes.h b/onnxruntime/core/providers/cpu/nn/conv_transpose_attributes.h index a1b1848ae1d08..b8c51d4cc966d 100644 --- a/onnxruntime/core/providers/cpu/nn/conv_transpose_attributes.h +++ b/onnxruntime/core/providers/cpu/nn/conv_transpose_attributes.h @@ -176,8 +176,8 @@ struct ConvTransposeAttributes : public ConvAttributes { void DistributePadding(AutoPadType pad_type, const int64_t& total_pad, int64_t& pad_head, int64_t& pad_tail) const { - // TODO (#9740) ORT 1.13 we will correct the logic in the following lines - LOGS_DEFAULT(WARNING) << "The pad result for SAME_UPPER and SAME_LOWER will be corrected in next ORT 1.13."; + // TODO (#9740) ORT 1.13 will correct the logic by switching them to meet ONNX spec + LOGS_DEFAULT(WARNING) << "The pad result for SAME_UPPER and SAME_LOWER will be corrected in next ORT 1.13 release."; if (pad_type == AutoPadType::SAME_UPPER) { // pad more on head when total_pad is odd. pad_head = total_pad - total_pad / 2; pad_tail = total_pad / 2; From a7963cf4dbb83d632ddff4e814b4d486e2c9a603 Mon Sep 17 00:00:00 2001 From: jcwchen Date: Fri, 24 Jun 2022 11:48:55 -0700 Subject: [PATCH 3/8] update TODO to make lint happy --- onnxruntime/core/providers/cpu/nn/conv_transpose_attributes.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/onnxruntime/core/providers/cpu/nn/conv_transpose_attributes.h b/onnxruntime/core/providers/cpu/nn/conv_transpose_attributes.h index b8c51d4cc966d..2533b20813b73 100644 --- a/onnxruntime/core/providers/cpu/nn/conv_transpose_attributes.h +++ b/onnxruntime/core/providers/cpu/nn/conv_transpose_attributes.h @@ -176,7 +176,7 @@ struct ConvTransposeAttributes : public ConvAttributes { void DistributePadding(AutoPadType pad_type, const int64_t& total_pad, int64_t& pad_head, int64_t& pad_tail) const { - // TODO (#9740) ORT 1.13 will correct the logic by switching them to meet ONNX spec + // TODO(jcwchen): #9740 ORT 1.13 will correct the logic by switching them to meet ONNX spec LOGS_DEFAULT(WARNING) << "The pad result for SAME_UPPER and SAME_LOWER will be corrected in next ORT 1.13 release."; if (pad_type == AutoPadType::SAME_UPPER) { // pad more on head when total_pad is odd. pad_head = total_pad - total_pad / 2; From 77f717a239b294416721b3087d5d67b097779b0a Mon Sep 17 00:00:00 2001 From: jcwchen Date: Fri, 24 Jun 2022 15:20:16 -0700 Subject: [PATCH 4/8] update more contents for warning and add if --- .../core/providers/cpu/nn/conv_transpose_attributes.h | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/onnxruntime/core/providers/cpu/nn/conv_transpose_attributes.h b/onnxruntime/core/providers/cpu/nn/conv_transpose_attributes.h index 2533b20813b73..80dc3b88f316a 100644 --- a/onnxruntime/core/providers/cpu/nn/conv_transpose_attributes.h +++ b/onnxruntime/core/providers/cpu/nn/conv_transpose_attributes.h @@ -176,13 +176,17 @@ struct ConvTransposeAttributes : public ConvAttributes { void DistributePadding(AutoPadType pad_type, const int64_t& total_pad, int64_t& pad_head, int64_t& pad_tail) const { - // TODO(jcwchen): #9740 ORT 1.13 will correct the logic by switching them to meet ONNX spec - LOGS_DEFAULT(WARNING) << "The pad result for SAME_UPPER and SAME_LOWER will be corrected in next ORT 1.13 release."; + if (pad_type != AutoPadType::NOTSET) { + // TODO(jcwchen): #9740 ORT 1.13 will correct the logic by switching them to meet ONNX spec + LOGS_DEFAULT(WARNING) << "The existing bug in the padding distribution for auto_pad type" + << " SAME_UPPER/SAME_LOWER/VALID will be fixed in next ORT 1.13 release and hence the" + << " results of ConvTranspose operator using the above auto_pad type(s) will be different."; + } if (pad_type == AutoPadType::SAME_UPPER) { // pad more on head when total_pad is odd. pad_head = total_pad - total_pad / 2; pad_tail = total_pad / 2; } else { - // for pad_type is NOTSET, SAME_LOWER or VALID + // for pad_type is SAME_LOWER or VALID // set pad_head as total_pad/2, pad_tail as total_pad-total_pad/2. // That said, we pad more on tail when total_pad is odd. pad_head = total_pad / 2; From 9b10e523a841fc88330f56d035a286d372875e26 Mon Sep 17 00:00:00 2001 From: jcwchen Date: Fri, 24 Jun 2022 15:39:30 -0700 Subject: [PATCH 5/8] valid was not infected --- .../core/providers/cpu/nn/conv_transpose_attributes.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/onnxruntime/core/providers/cpu/nn/conv_transpose_attributes.h b/onnxruntime/core/providers/cpu/nn/conv_transpose_attributes.h index 80dc3b88f316a..01b25e4b5cb67 100644 --- a/onnxruntime/core/providers/cpu/nn/conv_transpose_attributes.h +++ b/onnxruntime/core/providers/cpu/nn/conv_transpose_attributes.h @@ -176,17 +176,17 @@ struct ConvTransposeAttributes : public ConvAttributes { void DistributePadding(AutoPadType pad_type, const int64_t& total_pad, int64_t& pad_head, int64_t& pad_tail) const { - if (pad_type != AutoPadType::NOTSET) { + if (pad_type == AutoPadType::SAME_UPPER || pad_type == AutoPadType::SAME_LOWER) { // TODO(jcwchen): #9740 ORT 1.13 will correct the logic by switching them to meet ONNX spec LOGS_DEFAULT(WARNING) << "The existing bug in the padding distribution for auto_pad type" - << " SAME_UPPER/SAME_LOWER/VALID will be fixed in next ORT 1.13 release and hence the" + << " SAME_UPPER/SAME_LOWER will be fixed in next ORT 1.13 release and hence the" << " results of ConvTranspose operator using the above auto_pad type(s) will be different."; } if (pad_type == AutoPadType::SAME_UPPER) { // pad more on head when total_pad is odd. pad_head = total_pad - total_pad / 2; pad_tail = total_pad / 2; } else { - // for pad_type is SAME_LOWER or VALID + // for pad_type is SAME_LOWER // set pad_head as total_pad/2, pad_tail as total_pad-total_pad/2. // That said, we pad more on tail when total_pad is odd. pad_head = total_pad / 2; From cca0c016e970bbba8482dd3c2b4cc696c7abb0b9 Mon Sep 17 00:00:00 2001 From: Chun-Wei Chen Date: Tue, 28 Jun 2022 17:47:02 -0700 Subject: [PATCH 6/8] move it into kernel registration --- onnxruntime/core/providers/cpu/nn/conv_transpose.h | 9 ++++++++- .../core/providers/cpu/nn/conv_transpose_attributes.h | 8 +------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/onnxruntime/core/providers/cpu/nn/conv_transpose.h b/onnxruntime/core/providers/cpu/nn/conv_transpose.h index 49c174969b38b..3e42f1337c1a7 100644 --- a/onnxruntime/core/providers/cpu/nn/conv_transpose.h +++ b/onnxruntime/core/providers/cpu/nn/conv_transpose.h @@ -25,7 +25,14 @@ namespace onnxruntime { template class ConvTranspose : public OpKernel { public: - ConvTranspose(const OpKernelInfo& info) : OpKernel(info), conv_transpose_attrs_(info) {} + ConvTranspose(const OpKernelInfo& info) : OpKernel(info), conv_transpose_attrs_(info) { + if (auto_pad == AutoPadType::SAME_UPPER || auto_pad == AutoPadType::SAME_LOWER) { + // TODO(jcwchen): #9740 ORT 1.13 will correct the logic by switching them to meet ONNX spec + LOGS_DEFAULT(WARNING) << "The existing bug in the padding distribution for auto_pad type" + << " SAME_UPPER/SAME_LOWER will be fixed in next ORT 1.13 release and hence the" + << " results of ConvTranspose operator using the above auto_pad type(s) will be different."; + } + } Status PrePack(const Tensor& tensor, int input_idx, AllocatorPtr alloc, /*out*/ bool& is_packed, diff --git a/onnxruntime/core/providers/cpu/nn/conv_transpose_attributes.h b/onnxruntime/core/providers/cpu/nn/conv_transpose_attributes.h index 01b25e4b5cb67..ee74877da8d86 100644 --- a/onnxruntime/core/providers/cpu/nn/conv_transpose_attributes.h +++ b/onnxruntime/core/providers/cpu/nn/conv_transpose_attributes.h @@ -176,17 +176,11 @@ struct ConvTransposeAttributes : public ConvAttributes { void DistributePadding(AutoPadType pad_type, const int64_t& total_pad, int64_t& pad_head, int64_t& pad_tail) const { - if (pad_type == AutoPadType::SAME_UPPER || pad_type == AutoPadType::SAME_LOWER) { - // TODO(jcwchen): #9740 ORT 1.13 will correct the logic by switching them to meet ONNX spec - LOGS_DEFAULT(WARNING) << "The existing bug in the padding distribution for auto_pad type" - << " SAME_UPPER/SAME_LOWER will be fixed in next ORT 1.13 release and hence the" - << " results of ConvTranspose operator using the above auto_pad type(s) will be different."; - } if (pad_type == AutoPadType::SAME_UPPER) { // pad more on head when total_pad is odd. pad_head = total_pad - total_pad / 2; pad_tail = total_pad / 2; } else { - // for pad_type is SAME_LOWER + // for pad_type is NOTSET, SAME_LOWER or VALID // set pad_head as total_pad/2, pad_tail as total_pad-total_pad/2. // That said, we pad more on tail when total_pad is odd. pad_head = total_pad / 2; From b1d84305aaa3fe3b6598c9523741da000c1c4148 Mon Sep 17 00:00:00 2001 From: Chun-Wei Chen Date: Tue, 28 Jun 2022 18:23:41 -0700 Subject: [PATCH 7/8] parse auto_pad myself --- onnxruntime/core/providers/cpu/nn/conv_transpose.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/onnxruntime/core/providers/cpu/nn/conv_transpose.h b/onnxruntime/core/providers/cpu/nn/conv_transpose.h index 3e42f1337c1a7..3f160c5b69901 100644 --- a/onnxruntime/core/providers/cpu/nn/conv_transpose.h +++ b/onnxruntime/core/providers/cpu/nn/conv_transpose.h @@ -26,6 +26,12 @@ template class ConvTranspose : public OpKernel { public: ConvTranspose(const OpKernelInfo& info) : OpKernel(info), conv_transpose_attrs_(info) { + AutoPadType auto_pad = AutoPadType::NOTSET; + std::string auto_pad_str; + auto status = info.GetAttr("auto_pad", &auto_pad_str); + if (status.IsOK()) { + auto_pad = StringToAutoPadType(auto_pad_str); + } if (auto_pad == AutoPadType::SAME_UPPER || auto_pad == AutoPadType::SAME_LOWER) { // TODO(jcwchen): #9740 ORT 1.13 will correct the logic by switching them to meet ONNX spec LOGS_DEFAULT(WARNING) << "The existing bug in the padding distribution for auto_pad type" From 3b6fb871dcc29a8790bcbced80845c1919c919df Mon Sep 17 00:00:00 2001 From: Chun-Wei Chen Date: Tue, 28 Jun 2022 18:31:24 -0700 Subject: [PATCH 8/8] try to use conv_transpose_attrs_.auto_pad directly --- onnxruntime/core/providers/cpu/nn/conv_transpose.h | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/onnxruntime/core/providers/cpu/nn/conv_transpose.h b/onnxruntime/core/providers/cpu/nn/conv_transpose.h index 3f160c5b69901..53025e3b13013 100644 --- a/onnxruntime/core/providers/cpu/nn/conv_transpose.h +++ b/onnxruntime/core/providers/cpu/nn/conv_transpose.h @@ -26,13 +26,8 @@ template class ConvTranspose : public OpKernel { public: ConvTranspose(const OpKernelInfo& info) : OpKernel(info), conv_transpose_attrs_(info) { - AutoPadType auto_pad = AutoPadType::NOTSET; - std::string auto_pad_str; - auto status = info.GetAttr("auto_pad", &auto_pad_str); - if (status.IsOK()) { - auto_pad = StringToAutoPadType(auto_pad_str); - } - if (auto_pad == AutoPadType::SAME_UPPER || auto_pad == AutoPadType::SAME_LOWER) { + if (conv_transpose_attrs_.auto_pad == AutoPadType::SAME_UPPER || + conv_transpose_attrs_.auto_pad == AutoPadType::SAME_LOWER) { // TODO(jcwchen): #9740 ORT 1.13 will correct the logic by switching them to meet ONNX spec LOGS_DEFAULT(WARNING) << "The existing bug in the padding distribution for auto_pad type" << " SAME_UPPER/SAME_LOWER will be fixed in next ORT 1.13 release and hence the"