From e6195343d7c7a8d31d63fd862b73371ea2598ce5 Mon Sep 17 00:00:00 2001 From: Harsh Chauhan Date: Sat, 4 Apr 2026 22:23:41 +0530 Subject: [PATCH 1/2] [tmva][sofie] fix SAME_UPPER/SAME_LOWER autopad padding formula in Conv and Pool --- tmva/sofie/inc/TMVA/ROperator_Conv.hxx | 48 +++++++++--------- tmva/sofie/inc/TMVA/ROperator_Pool.hxx | 45 ++++++++-------- tmva/sofie/test/TestCustomModelsFromONNX.cxx | 23 +++++++++ .../ConvWithAutopadSameUpper.onnx | Bin 0 -> 222 bytes .../ConvWithAutopadSameUpper.ref.hxx | 3 ++ 5 files changed, 72 insertions(+), 47 deletions(-) create mode 100644 tmva/sofie/test/input_models/ConvWithAutopadSameUpper.onnx create mode 100644 tmva/sofie/test/input_models/references/ConvWithAutopadSameUpper.ref.hxx diff --git a/tmva/sofie/inc/TMVA/ROperator_Conv.hxx b/tmva/sofie/inc/TMVA/ROperator_Conv.hxx index b4a2bb547c3bd..d604ef6742cde 100644 --- a/tmva/sofie/inc/TMVA/ROperator_Conv.hxx +++ b/tmva/sofie/inc/TMVA/ROperator_Conv.hxx @@ -137,28 +137,37 @@ public: k2 + (fAttrDilations[1] - 1) * (k2 - 1), k3 + (fAttrDilations[2] - 1) * (k3 - 1)}; + if (fAttrStrides.empty()) { + fAttrStrides = {1, 1, 1}; + } + if (fDim < 3) + fAttrStrides.resize(3, 1); + if (fAttrAutopad == "NOTSET") { if (fAttrPads.empty()) { fAttrPads = {1, 1, 1, 1, 1, 1}; } } else if (fAttrAutopad == "SAME_UPPER" || fAttrAutopad == "SAME_LOWER") { - if (fDim == 1) - fAttrPads = {fAttrKernelShape[0] / 2, fAttrKernelShape[0] / 2}; - else if (fDim == 2) - fAttrPads = {fAttrKernelShape[0] / 2, fAttrKernelShape[1] / 2, fAttrKernelShape[0] / 2, fAttrKernelShape[1] / 2}; - else if (fDim == 3) - fAttrPads = {fAttrKernelShape[0] / 2, fAttrKernelShape[1] / 2, fAttrKernelShape[2] / 2, - fAttrKernelShape[0] / 2, fAttrKernelShape[1] / 2, fAttrKernelShape[2] / 2}; - // add extra padding at beginning or end (depending if SAME_UPPER or SAME_LOWER) - // need to check this! - if (fAttrKernelShape[0] % 2 == 1) { - (fAttrAutopad == "SAME_UPPER") ? fAttrPads[0]++ : fAttrPads[i1]++; + for (size_t d = 0; d < fDim; ++d) { + if (input[d + 2].isParam) + throw std::runtime_error( + "TMVA SOFIE Conv Op: SAME padding with parametric input shape is not supported"); } - if (fDim > 1 && fAttrKernelShape[1] % 2 == 1) { - (fAttrAutopad == "SAME_UPPER") ? fAttrPads[1]++ : fAttrPads[i2]++; - } - if (fDim > 2 && fAttrKernelShape[2] % 2 == 1) { - (fAttrAutopad == "SAME_UPPER") ? fAttrPads[2]++ : fAttrPads[i3]++; + // ONNX SAME padding: total_pad = max(0, (ceil(in/stride)-1)*stride + kernel - in) + // SAME_UPPER places extra padding at end, SAME_LOWER at beginning + fAttrPads.assign(6, 0); + for (size_t d = 0; d < fDim; ++d) { + size_t inSize = input[d + 2].dim; + size_t stride_d = fAttrStrides[d]; + size_t outSize = (inSize + stride_d - 1) / stride_d; + int totalPad = std::max(0, (int)((outSize - 1) * stride_d + fAttrKernelShape[d]) - (int)inSize); + if (fAttrAutopad == "SAME_UPPER") { + fAttrPads[d] = (size_t)(totalPad / 2); + fAttrPads[d + fDim] = (size_t)(totalPad - totalPad / 2); + } else { + fAttrPads[d] = (size_t)(totalPad - totalPad / 2); + fAttrPads[d + fDim] = (size_t)(totalPad / 2); + } } } else if (fAttrAutopad != "VALID") { throw @@ -167,13 +176,6 @@ public: // to be sure pad is vector of size 6 if (fDim < 3) fAttrPads.resize(6, 0); - if (fAttrStrides.empty()) { - fAttrStrides = {1, 1, 1}; - } - if (fDim < 3) - fAttrStrides.resize(3, 1); - - Dim input1 = input[2]; Dim input2 = (fDim > 1) ? input[3] : Dim{1}; Dim input3 = (fDim > 2) ? input[4] : Dim{1}; diff --git a/tmva/sofie/inc/TMVA/ROperator_Pool.hxx b/tmva/sofie/inc/TMVA/ROperator_Pool.hxx index febf49d416686..de5998255f1bf 100644 --- a/tmva/sofie/inc/TMVA/ROperator_Pool.hxx +++ b/tmva/sofie/inc/TMVA/ROperator_Pool.hxx @@ -132,29 +132,33 @@ public: k2 + (fAttrDilations[1] - 1) * (k2 - 1), k3 + (fAttrDilations[2] - 1) * (k3 - 1)}; + if (fAttrStrides.empty()) { + fAttrStrides = {1, 1, 1}; + } + if (fDim < 3) + fAttrStrides.resize(3, 1); + if (fAttrAutopad == "NOTSET") { // in auto_pad is NOTSET then fAttrPads should have been set or default zero is used if (fAttrPads.empty()) { fAttrPads = {0, 0, 0, 0, 0, 0}; } } else if (fAttrAutopad == "SAME_UPPER" || fAttrAutopad == "SAME_LOWER") { - if (fDim == 1) - fAttrPads = {fAttrKernelShape[0] / 2, fAttrKernelShape[0] / 2}; - else if (fDim == 2) - fAttrPads = {fAttrKernelShape[0] / 2, fAttrKernelShape[1] / 2, fAttrKernelShape[0] / 2, fAttrKernelShape[1] / 2}; - else if (fDim == 3) - fAttrPads = {fAttrKernelShape[0] / 2, fAttrKernelShape[1] / 2, fAttrKernelShape[2] / 2, - fAttrKernelShape[0] / 2, fAttrKernelShape[1] / 2, fAttrKernelShape[2] / 2}; - // add extra padding at beginning or end (depending if SAME_UPPER or SAME_LOWER) - // need to check this! - if (fAttrKernelShape[0] % 2 == 1) { - (fAttrAutopad == "SAME_UPPER") ? fAttrPads[0]++ : fAttrPads[i1]++; - } - if (fDim > 1 && fAttrKernelShape[1] % 2 == 1) { - (fAttrAutopad == "SAME_UPPER") ? fAttrPads[1]++ : fAttrPads[i2]++; - } - if (fDim > 2 && fAttrKernelShape[2] % 2 == 1) { - (fAttrAutopad == "SAME_UPPER") ? fAttrPads[2]++ : fAttrPads[i3]++; + // ONNX SAME padding: total_pad = max(0, (ceil(in/stride)-1)*stride + kernel - in) + // SAME_UPPER places extra padding at end, SAME_LOWER at beginning + fAttrPads.assign(6, 0); + for (size_t d = 0; d < fDim; ++d) { + size_t inSize = input[0][d + 2]; + size_t stride_d = fAttrStrides[d]; + size_t outSize = (inSize + stride_d - 1) / stride_d; + int totalPad = std::max(0, (int)((outSize - 1) * stride_d + fAttrKernelShape[d]) - (int)inSize); + if (fAttrAutopad == "SAME_UPPER") { + fAttrPads[d] = (size_t)(totalPad / 2); + fAttrPads[d + fDim] = (size_t)(totalPad - totalPad / 2); + } else { + fAttrPads[d] = (size_t)(totalPad - totalPad / 2); + fAttrPads[d + fDim] = (size_t)(totalPad / 2); + } } } else if (fAttrAutopad != "VALID") { throw @@ -163,13 +167,6 @@ public: // to be sure pad is vector of size 6 if (fDim < 3) fAttrPads.resize(6, 0); - if (fAttrStrides.empty()) { - fAttrStrides = {1, 1, 1}; - } - - if (fDim < 3) - fAttrStrides.resize(3, 1); - size_t input1 = input[0][2]; size_t input2 = (fDim > 1) ? input[0][3] : 1; size_t input3 = (fDim > 2) ? input[0][4] : 1; diff --git a/tmva/sofie/test/TestCustomModelsFromONNX.cxx b/tmva/sofie/test/TestCustomModelsFromONNX.cxx index 94993f601a3c4..b84847d03adca 100644 --- a/tmva/sofie/test/TestCustomModelsFromONNX.cxx +++ b/tmva/sofie/test/TestCustomModelsFromONNX.cxx @@ -24,6 +24,7 @@ constexpr auto modelDataSuffix = "_FromONNX.dat"; #include "input_models/references/ConvWithPadding.ref.hxx" #include "input_models/references/ConvWithoutPadding.ref.hxx" #include "input_models/references/ConvWithAutopadSameLower.ref.hxx" +#include "input_models/references/ConvWithAutopadSameUpper.ref.hxx" #include "input_models/references/ConvWithStridesPadding.ref.hxx" #include "input_models/references/ConvWithStridesNoPadding.ref.hxx" #include "input_models/references/ConvWithAsymmetricPadding.ref.hxx" @@ -664,6 +665,28 @@ TEST(ONNX, ConvWithAutopadSameLower) } +TEST(ONNX, ConvWithAutopadSameUpper) +{ + constexpr float TOLERANCE = DEFAULT_TOLERANCE; + + // Input (1,1,5,5) with values 0..24; kernel (1,1,3,3) all-ones, auto_pad=SAME_UPPER, stride=1 + // Odd kernel: total_pad=2 per dim, begin=1 end=1 (symmetric); output shape (1,1,5,5) + std::vector input(25); + std::iota(input.begin(), input.end(), 0.0f); + ASSERT_INCLUDE_AND_RUN(std::vector, "ConvWithAutopadSameUpper", input); + + // Checking output size + EXPECT_EQ(output.size(), sizeof(ConvWithAutopadSameUpper_ExpectedOutput::output) / sizeof(float)); + + float *correct = ConvWithAutopadSameUpper_ExpectedOutput::output; + + // Checking every output value, one by one + for (size_t i = 0; i < output.size(); ++i) { + EXPECT_LE(std::abs(output[i] - correct[i]), TOLERANCE); + } +} + + TEST(ONNX, ConvWithStridesPadding) { constexpr float TOLERANCE = DEFAULT_TOLERANCE; diff --git a/tmva/sofie/test/input_models/ConvWithAutopadSameUpper.onnx b/tmva/sofie/test/input_models/ConvWithAutopadSameUpper.onnx new file mode 100644 index 0000000000000000000000000000000000000000..1dade4fb31d2368673edfc9e8d6ca1a76ed46d3a GIT binary patch literal 222 zcmdYOw>O10zsGh!rkFbgm$sW31!*b~A|jNwtzK!*v5afxs+3JGvAaWFzLE0AVQ0!miml5}F> IVi4d40LMxwiU0rr literal 0 HcmV?d00001 diff --git a/tmva/sofie/test/input_models/references/ConvWithAutopadSameUpper.ref.hxx b/tmva/sofie/test/input_models/references/ConvWithAutopadSameUpper.ref.hxx new file mode 100644 index 0000000000000..948451c3db152 --- /dev/null +++ b/tmva/sofie/test/input_models/references/ConvWithAutopadSameUpper.ref.hxx @@ -0,0 +1,3 @@ +namespace ConvWithAutopadSameUpper_ExpectedOutput { +float output[] = {12., 21., 27., 33., 24., 33., 54., 63., 72., 51., 63., 99., 108., 117., 81., 93., 144., 153., 162., 111., 72., 111., 117., 123., 84.}; +} // namespace ConvWithAutopadSameUpper_ExpectedOutput From a8dfc0e92c00e14572b474575134bd0b4dcb0ce3 Mon Sep 17 00:00:00 2001 From: Harsh Chauhan Date: Wed, 8 Apr 2026 19:47:04 +0530 Subject: [PATCH 2/2] clang format fix --- tmva/sofie/test/TestCustomModelsFromONNX.cxx | 2 -- .../input_models/references/ConvWithAutopadSameUpper.ref.hxx | 3 ++- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/tmva/sofie/test/TestCustomModelsFromONNX.cxx b/tmva/sofie/test/TestCustomModelsFromONNX.cxx index b84847d03adca..8869bc72cf6b1 100644 --- a/tmva/sofie/test/TestCustomModelsFromONNX.cxx +++ b/tmva/sofie/test/TestCustomModelsFromONNX.cxx @@ -664,7 +664,6 @@ TEST(ONNX, ConvWithAutopadSameLower) } } - TEST(ONNX, ConvWithAutopadSameUpper) { constexpr float TOLERANCE = DEFAULT_TOLERANCE; @@ -686,7 +685,6 @@ TEST(ONNX, ConvWithAutopadSameUpper) } } - TEST(ONNX, ConvWithStridesPadding) { constexpr float TOLERANCE = DEFAULT_TOLERANCE; diff --git a/tmva/sofie/test/input_models/references/ConvWithAutopadSameUpper.ref.hxx b/tmva/sofie/test/input_models/references/ConvWithAutopadSameUpper.ref.hxx index 948451c3db152..71bf99b6cb987 100644 --- a/tmva/sofie/test/input_models/references/ConvWithAutopadSameUpper.ref.hxx +++ b/tmva/sofie/test/input_models/references/ConvWithAutopadSameUpper.ref.hxx @@ -1,3 +1,4 @@ namespace ConvWithAutopadSameUpper_ExpectedOutput { -float output[] = {12., 21., 27., 33., 24., 33., 54., 63., 72., 51., 63., 99., 108., 117., 81., 93., 144., 153., 162., 111., 72., 111., 117., 123., 84.}; +float output[] = {12., 21., 27., 33., 24., 33., 54., 63., 72., 51., 63., 99., 108., + 117., 81., 93., 144., 153., 162., 111., 72., 111., 117., 123., 84.}; } // namespace ConvWithAutopadSameUpper_ExpectedOutput