From a9fc24e9a1dee164e822e980600aaed601d84aaa Mon Sep 17 00:00:00 2001 From: Trevor Morris Date: Mon, 20 May 2019 13:25:20 -0700 Subject: [PATCH 1/4] Avoid calculating padding when it is not necessary i.e. TRT 5.1.3+ --- .../tf2tensorrt/convert/convert_nodes.cc | 34 +++++++------------ 1 file changed, 13 insertions(+), 21 deletions(-) diff --git a/tensorflow/compiler/tf2tensorrt/convert/convert_nodes.cc b/tensorflow/compiler/tf2tensorrt/convert/convert_nodes.cc index 306341301b9c97..00fe2ab7444fee 100644 --- a/tensorflow/compiler/tf2tensorrt/convert/convert_nodes.cc +++ b/tensorflow/compiler/tf2tensorrt/convert/convert_nodes.cc @@ -1950,7 +1950,8 @@ Status ConvertConv2DHelper(OpConverterParams* params, int group, kernel_size.h() = weights.shape_.d[2]; kernel_size.w() = weights.shape_.d[3]; - // Add padding. +// Before TRT 5.1.3, we have to calculate padding ourselves. +#if !IS_TRT_VERSION_GE(5, 1, 3, 0) std::vector> padding; if (attrs.get("padding") == "SAME") { nvinfer1::DimsHW effective_kernel_size = kernel_size; @@ -1977,9 +1978,8 @@ Status ConvertConv2DHelper(OpConverterParams* params, int group, padding = {{0, 0}, {0, 0}}; } -// TensorRT 5.1 added support for asymmetric padding. Due to a bug in 5.1.2, we -// can only use asymmetric padding in convolutions with 5.1.3+. -#if !IS_TRT_VERSION_GE(5, 1, 3, 0) + // TensorRT 5.1 added support for asymmetric padding. Due to a bug in 5.1.2, + // we can only use asymmetric padding in convolutions with 5.1.3+. if (padding[0].first != padding[0].second || padding[1].first != padding[1].second) { // Handle asymmetric padding. @@ -2005,17 +2005,12 @@ Status ConvertConv2DHelper(OpConverterParams* params, int group, layer->setStride(stride); // TensorRT 5.1.3 added support for padding modes. #if IS_TRT_VERSION_GE(5, 1, 3, 0) + // VALID padding is the default TRT behavior. if (attrs.get("padding") == "SAME") { VLOG(2) << "Using SAME padding"; // SAME_UPPER means that post padding is preferred. layer->setPaddingMode(nvinfer1::PaddingMode::kSAME_UPPER); } - // For VALID padding, we need to manually set the padding. - layer->setPrePadding(nvinfer1::DimsHW{padding[0].first, padding[1].first}); - layer->setPostPadding( - nvinfer1::DimsHW{padding[0].second, padding[1].second}); - VLOG(2) << "Set pre-padding to: " << DebugString(layer->getPrePadding()) - << " and post-padding to: " << DebugString(layer->getPostPadding()); #else layer->setPadding(nvinfer1::DimsHW{padding[0].first, padding[1].first}); VLOG(2) << "Set padding to: " << DebugString(layer->getPadding()); @@ -2035,11 +2030,6 @@ Status ConvertConv2DHelper(OpConverterParams* params, int group, VLOG(2) << "Using SAME padding"; layer->setPaddingMode(nvinfer1::PaddingMode::kSAME_UPPER); } - layer->setPrePadding(nvinfer1::DimsHW{padding[0].first, padding[1].first}); - layer->setPostPadding( - nvinfer1::DimsHW{padding[0].second, padding[1].second}); - VLOG(2) << "Set pre-padding to: " << DebugString(layer->getPrePadding()) - << " and post-padding to: " << DebugString(layer->getPostPadding()); #else layer->setPadding(nvinfer1::DimsHW{padding[0].first, padding[1].first}); VLOG(2) << "Set padding to: " << DebugString(layer->getPadding()); @@ -2776,6 +2766,8 @@ Status ConvertPool(OpConverterParams* params) { const auto tf_kernel = attrs.get>("ksize"); const nvinfer1::DimsHW ksize(tf_kernel[h_index], tf_kernel[w_index]); +// Before TRT 5.1.3, we have to calculate padding ourselves. +#if !IS_TRT_VERSION_GE(5, 1, 3, 0) auto tensor_dim = tensor->getDimensions(); std::vector> padding; if (padding_type == "SAME") { @@ -2788,9 +2780,11 @@ Status ConvertPool(OpConverterParams* params) { } else if (padding_type == "VALID") { padding = {{0, 0}, {0, 0}}; } - -// TensorRT 5.1 added support for asymmetric padding. +#endif +// TensorRT 5.1 added support for asymmetric padding. Before that, we need an +// extra padding layer. #if !IS_TRT_VERSION_GE(5, 1, 0, 0) + // Asymmetric padding case. if (padding[0].first != padding[0].second || padding[1].first != padding[1].second) { VLOG(2) << "Padding!!!: " << padding[0].first << padding[0].second @@ -2818,14 +2812,12 @@ Status ConvertPool(OpConverterParams* params) { layer->setStride(stride); // TensorRT 5.1.3 added support for padding modes. #if IS_TRT_VERSION_GE(5, 1, 3, 0) + // VALID padding is the default TRT behavior. if (attrs.get("padding") == "SAME") { // SAME_UPPER means that post padding is preferred. layer->setPaddingMode(nvinfer1::PaddingMode::kSAME_UPPER); } -#endif -// TensorRT 5.1 has support for asymmetric padding. -#if IS_TRT_VERSION_GE(5, 1, 0, 0) - // If padding mode is not SAME, then these values will be used instead. +#elif IS_TRT_VERSION_GE(5, 1, 0, 0) layer->setPrePadding(nvinfer1::DimsHW{padding[0].first, padding[1].first}); layer->setPostPadding(nvinfer1::DimsHW{padding[0].second, padding[1].second}); #else From ed0a8b6b21ba708b6b29e7b3717f8892ee8b2414 Mon Sep 17 00:00:00 2001 From: Trevor Morris Date: Mon, 20 May 2019 13:26:03 -0700 Subject: [PATCH 2/4] Remove padding debug prints --- tensorflow/compiler/tf2tensorrt/convert/convert_nodes.cc | 6 ------ 1 file changed, 6 deletions(-) diff --git a/tensorflow/compiler/tf2tensorrt/convert/convert_nodes.cc b/tensorflow/compiler/tf2tensorrt/convert/convert_nodes.cc index 00fe2ab7444fee..e01bb24ab2e3f4 100644 --- a/tensorflow/compiler/tf2tensorrt/convert/convert_nodes.cc +++ b/tensorflow/compiler/tf2tensorrt/convert/convert_nodes.cc @@ -2007,13 +2007,11 @@ Status ConvertConv2DHelper(OpConverterParams* params, int group, #if IS_TRT_VERSION_GE(5, 1, 3, 0) // VALID padding is the default TRT behavior. if (attrs.get("padding") == "SAME") { - VLOG(2) << "Using SAME padding"; // SAME_UPPER means that post padding is preferred. layer->setPaddingMode(nvinfer1::PaddingMode::kSAME_UPPER); } #else layer->setPadding(nvinfer1::DimsHW{padding[0].first, padding[1].first}); - VLOG(2) << "Set padding to: " << DebugString(layer->getPadding()); #endif layer->setName(node_def.name().c_str()); layer->setNbGroups(num_groups); @@ -2027,12 +2025,10 @@ Status ConvertConv2DHelper(OpConverterParams* params, int group, layer->setStride(stride); #if IS_TRT_VERSION_GE(5, 1, 3, 0) if (attrs.get("padding") == "SAME") { - VLOG(2) << "Using SAME padding"; layer->setPaddingMode(nvinfer1::PaddingMode::kSAME_UPPER); } #else layer->setPadding(nvinfer1::DimsHW{padding[0].first, padding[1].first}); - VLOG(2) << "Set padding to: " << DebugString(layer->getPadding()); #endif layer->setName(node_def.name().c_str()); layer->setNbGroups(num_groups); @@ -2787,8 +2783,6 @@ Status ConvertPool(OpConverterParams* params) { // Asymmetric padding case. if (padding[0].first != padding[0].second || padding[1].first != padding[1].second) { - VLOG(2) << "Padding!!!: " << padding[0].first << padding[0].second - << padding[1].first << padding[1].second; auto pad_layer = params->converter->network()->addPadding( *tensor, nvinfer1::DimsHW(padding[0].first, padding[1].first), nvinfer1::DimsHW(padding[0].second, padding[1].second)); From 594db0fcedd1a2846aafb08d93f7ea604e3d39fe Mon Sep 17 00:00:00 2001 From: Trevor Morris Date: Mon, 20 May 2019 13:46:05 -0700 Subject: [PATCH 3/4] Formatting --- tensorflow/compiler/tf2tensorrt/convert/convert_nodes.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tensorflow/compiler/tf2tensorrt/convert/convert_nodes.cc b/tensorflow/compiler/tf2tensorrt/convert/convert_nodes.cc index e01bb24ab2e3f4..53fd39a3c76ffb 100644 --- a/tensorflow/compiler/tf2tensorrt/convert/convert_nodes.cc +++ b/tensorflow/compiler/tf2tensorrt/convert/convert_nodes.cc @@ -2780,7 +2780,7 @@ Status ConvertPool(OpConverterParams* params) { // TensorRT 5.1 added support for asymmetric padding. Before that, we need an // extra padding layer. #if !IS_TRT_VERSION_GE(5, 1, 0, 0) - // Asymmetric padding case. + // Asymmetric padding case. if (padding[0].first != padding[0].second || padding[1].first != padding[1].second) { auto pad_layer = params->converter->network()->addPadding( From 14438d5235ef08c32937fc275117392a6403539f Mon Sep 17 00:00:00 2001 From: Trevor Morris Date: Mon, 20 May 2019 13:57:23 -0700 Subject: [PATCH 4/4] Clarify why we aren't using setPrePadding/setPostPadding for conv --- tensorflow/compiler/tf2tensorrt/convert/convert_nodes.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tensorflow/compiler/tf2tensorrt/convert/convert_nodes.cc b/tensorflow/compiler/tf2tensorrt/convert/convert_nodes.cc index 53fd39a3c76ffb..6cb09cb122b2c3 100644 --- a/tensorflow/compiler/tf2tensorrt/convert/convert_nodes.cc +++ b/tensorflow/compiler/tf2tensorrt/convert/convert_nodes.cc @@ -1978,11 +1978,12 @@ Status ConvertConv2DHelper(OpConverterParams* params, int group, padding = {{0, 0}, {0, 0}}; } - // TensorRT 5.1 added support for asymmetric padding. Due to a bug in 5.1.2, - // we can only use asymmetric padding in convolutions with 5.1.3+. + // Handle asymmetric padding. TensorRT 5.1 added support for asymmetric + // padding via setPrePadding and setPostPadding. Due to a bug in 5.1.2, we can + // only use asymmetric padding in convolutions with 5.1.3+. But in 5.1.3, we + // will always use setPaddingMode for simplicity. if (padding[0].first != padding[0].second || padding[1].first != padding[1].second) { - // Handle asymmetric padding. auto pad_layer = params->converter->network()->addPadding( *tensor, nvinfer1::DimsHW(padding[0].first, padding[1].first), nvinfer1::DimsHW(padding[0].second, padding[1].second)); @@ -2804,7 +2805,6 @@ Status ConvertPool(OpConverterParams* params) { layer->getOutput(0)); layer->setStride(stride); -// TensorRT 5.1.3 added support for padding modes. #if IS_TRT_VERSION_GE(5, 1, 3, 0) // VALID padding is the default TRT behavior. if (attrs.get("padding") == "SAME") {