Check duplicate issues.
Description
SOFIE generates incorrect code for MaxPool (and AveragePool) whenever the padding is not symmetric across the spatial axes. The output values come out wrong, and in the asymmetric case the generated loop even produces a different output shape than the one shape inference computed.
The cause is an inconsistency in how the pads attribute is indexed inside ROperator_Pool. ONNX stores pads in grouped order, as [x0_begin, x1_begin, ..., x0_end, x1_end, ...], and SOFIE keeps them that way. The shape inference and the padded buffer allocation read them in this grouped order, so they are correct. The code that computes the pooling window bounds in Generate() reads the same array as if it were laid out per axis, as [begin, end, begin, end, ...].
For a 2D pool this swaps the height end pad with the width begin pad, so the window start and
limit are taken from the wrong entries.
Now bcz the bounds and the shape inference disagree about the padding; the generated loop can iterate a different number of output elements than the output tensor was sized for. In a small example (MaxPool 2x2, stride 1, pads [0,1,0,1]) the loop builds a 4 by 4 result and writes it into a buffer allocated for 3 by 5. That is both wrong values and a write past the end of the output. Nothing throws and no warning is printed.
However the bug only shows up with asymmetric padding. Every pool model in the SOFIE test suite uses zero padding, where the wrong index happens to read the same zero, so the path has never been exercised.
By contrast, the Conv op reads the same attribute in grouped order and handles asymmetric padding correctly (the ConvWithAsymmetricPadding test passes), which confirms grouped is the intended layout.
Expected: the generated MaxPool and AveragePool should match the ONNX reference output for any valid padding, the same way Conv already does.
Reproducer
Only ROOT (with SOFIE) and the onnx python package are needed. Step 1 builds the exact model used. The bug shows up in SOFIE's generated code,so inference isnt required.
- Build the MaxPool model. It pads the width axis only (
pads=[0,1,0,1]), input
1x1x4x4:
# make_model.py
import onnx
from onnx import helper, TensorProto
X = helper.make_tensor_value_info("X", TensorProto.FLOAT, [1, 1, 4, 4])
Y = helper.make_tensor_value_info("Y", TensorProto.FLOAT, [1, 1, 3, 5])
node = helper.make_node("MaxPool", ["X"], ["Y"],
kernel_shape=[2, 2], strides=[1, 1], pads=[0, 1, 0, 1])
m = helper.make_model(helper.make_graph([node], "maxpool_asym", [X], [Y]),
opset_imports=[helper.make_opsetid("", 13)])
onnx.checker.check_model(m)
onnx.save(m, "maxpool_asym.onnx")
- Generate the SOFIE code:
// gen.C
#include "TMVA/RModelParser_ONNX.hxx"
#include "TMVA/RModel.hxx"
void gen() {
using namespace TMVA::Experimental::SOFIE;
RModel model = RModelParser_ONNX().Parse("maxpool_asym.onnx");
model.Generate();
model.OutputGenerated("maxpool_asym_generated.hxx");
}
- Read the pooling window bounds from the generated header:
grep -nE 'constexpr int (hmin|hmax|wmin|wmax)' maxpool_asym_generated.hxx
Observed:
hmin = 0
hmax = 4
wmin = 0
wmax = 4
For pads=[0,1,0,1] the correct bounds are hmax = 3 and wmin = -1. So the width padding is dropped (wmin is 0 not -1) and the height loop runs one row too far (hmax is 4 not 3). The loop then builds a 4 by 4 result into the 1x1x3x5 output buffer that shape inference allocated. With symmetric padding (for example pads=[1,1,1,1]) the bounds are correct, which is why the existing zero padding pool tests miss it.
ROOT version
ROOT 6.41.01
Installation method
Built from source
Operating system
Ubuntu 22.04.2 LTS
Additional context
No response
Check duplicate issues.
Description
SOFIE generates incorrect code for MaxPool (and AveragePool) whenever the padding is not symmetric across the spatial axes. The output values come out wrong, and in the asymmetric case the generated loop even produces a different output shape than the one shape inference computed.
The cause is an inconsistency in how the
padsattribute is indexed inside ROperator_Pool. ONNX stores pads in grouped order, as[x0_begin, x1_begin, ..., x0_end, x1_end, ...], and SOFIE keeps them that way. The shape inference and the padded buffer allocation read them in this grouped order, so they are correct. The code that computes the pooling window bounds inGenerate()reads the same array as if it were laid out per axis, as [begin, end, begin, end, ...].For a 2D pool this swaps the height end pad with the width begin pad, so the window start and
limit are taken from the wrong entries.
Now bcz the bounds and the shape inference disagree about the padding; the generated loop can iterate a different number of output elements than the output tensor was sized for. In a small example (MaxPool 2x2, stride 1, pads [0,1,0,1]) the loop builds a 4 by 4 result and writes it into a buffer allocated for 3 by 5. That is both wrong values and a write past the end of the output. Nothing throws and no warning is printed.
However the bug only shows up with asymmetric padding. Every pool model in the SOFIE test suite uses zero padding, where the wrong index happens to read the same zero, so the path has never been exercised.
By contrast, the Conv op reads the same attribute in grouped order and handles asymmetric padding correctly (the ConvWithAsymmetricPadding test passes), which confirms grouped is the intended layout.
Expected: the generated MaxPool and AveragePool should match the ONNX reference output for any valid padding, the same way Conv already does.
Reproducer
Only ROOT (with SOFIE) and the
onnxpython package are needed. Step 1 builds the exact model used. The bug shows up in SOFIE's generated code,so inference isnt required.pads=[0,1,0,1]), input1x1x4x4:
Observed:
For
pads=[0,1,0,1]the correct bounds arehmax = 3andwmin = -1. So the width padding is dropped (wminis 0 not -1) and the height loop runs one row too far (hmaxis 4 not 3). The loop then builds a 4 by 4 result into the 1x1x3x5 output buffer that shape inference allocated. With symmetric padding (for examplepads=[1,1,1,1]) the bounds are correct, which is why the existing zero padding pool tests miss it.ROOT version
ROOT 6.41.01
Installation method
Built from source
Operating system
Ubuntu 22.04.2 LTS
Additional context
No response