Skip to content

Commit d42a7a7

Browse files
authored
[tmva][sofie] Fix double-counted dilation in Conv im2col
Conv generated invalid code and crashed (segfault) for dilation > 1. The operator builds a dilated `_f` weight buffer and expands `fAttrKernelShape` to the effective kernel size, but the im2col call was still passed the real dilation, so dilation was applied twice (expanded kernel + dilation). This produced a negative output dimension and an out-of-bounds write. Fix: after the dilated `_f` is built, the dense im2col must use dilation 1, since the dilation is already folded into the expanded kernel shape and the `_f` layout. This is a no-op for dilation 1, so existing Conv tests are unaffected. Also adds a `ConvWithDilation` test (3x3 kernel, dilation 2) with reference output. The suite had no dilation > 1 case before, which is why this was never caught. The full SOFIE test suite passes locally on master.
1 parent af14fa6 commit d42a7a7

4 files changed

Lines changed: 29 additions & 0 deletions

File tree

tmva/sofie/inc/TMVA/ROperator_Conv.hxx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -429,6 +429,10 @@ public:
429429
out << SP << SP << "}\n";
430430
out << SP << "}\n";
431431

432+
// Dilation is already folded into the expanded kernel shape and the dilated tensor_<X>_f
433+
// layout above, so the dense im2col below must use dilation 1 to avoid double-counting it.
434+
fAttrDilations = std::vector<size_t>(3, 1);
435+
432436
//out << SP << "char " << OpName << "_transA = 'T';\n";
433437
out << SP << "char " << OpName << "_transA = 'N';\n";
434438
out << SP << "char " << OpName << "_transB = 'N';\n";

tmva/sofie/test/TestCustomModelsFromONNX.cxx

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ constexpr auto modelDataSuffix = "_FromONNX.dat";
2222
#include "input_models/references/Erf.ref.hxx"
2323
#include "input_models/references/LinearWithSigmoid.ref.hxx"
2424
#include "input_models/references/ConvWithPadding.ref.hxx"
25+
#include "input_models/references/ConvWithDilation.ref.hxx"
2526
#include "input_models/references/ConvWithoutPadding.ref.hxx"
2627
#include "input_models/references/ConvWithAutopadSameLower.ref.hxx"
2728
#include "input_models/references/ConvWithAutopadSameUpper.ref.hxx"
@@ -726,6 +727,27 @@ TEST(ONNX, ConvWithStridesPadding)
726727
}
727728

728729

730+
TEST(ONNX, ConvWithDilation)
731+
{
732+
constexpr float TOLERANCE = DEFAULT_TOLERANCE;
733+
734+
// Preparing the standard all-ones input
735+
std::vector<float> input(49);
736+
std::iota(input.begin(), input.end(), 0.0f);
737+
ASSERT_INCLUDE_AND_RUN(std::vector<float>, "ConvWithDilation", input);
738+
739+
// Checking output size
740+
EXPECT_EQ(output.size(), std::size(ConvWithDilation_ExpectedOutput::all_ones));
741+
742+
float *correct = ConvWithDilation_ExpectedOutput::all_ones;
743+
744+
// Checking every output value, one by one
745+
for (size_t i = 0; i < output.size(); ++i) {
746+
EXPECT_LE(std::abs(output[i] - correct[i]), TOLERANCE);
747+
}
748+
}
749+
750+
729751
TEST(ONNX, ConvWithStridesNoPadding)
730752
{
731753
constexpr float TOLERANCE = DEFAULT_TOLERANCE;
269 Bytes
Binary file not shown.
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
namespace ConvWithDilation_ExpectedOutput {
2+
float all_ones[] = {98.400002f, 102.900009f, 107.400002f, 129.899994f, 134.399994f, 138.899994f, 161.399994f, 165.899994f, 170.399994f};
3+
} // namespace ConvWithDilation_ExpectedOutput

0 commit comments

Comments
 (0)