Bad padding handling for PaddedMaxPooling

Aidge's PaddedMaxPooling is giving different results than ONNX or Pytorch's MaxPooling.
In Aidge, input tensor is padded then the zeroes are counted as values of the tensor, however, in Torch and ONNX, the padding seems to be just for maintaining spatial dimensions, the zeroes are not counted.
Here is the test script:

import onnx
import numpy as np
import onnxruntime as ort
from onnx import helper, TensorProto
import aidge_core; import aidge_backend_cpu; import aidge_onnx;
import torch
import torch.nn.functional as F

def create_maxpool_model(output_filename):
    """Creates an ONNX model with a MaxPool operation with padding"""
    input_tensor = helper.make_tensor_value_info('input', TensorProto.FLOAT, [1, 1, 4, 4])
    output_tensor = helper.make_tensor_value_info('output', TensorProto.FLOAT, [1, 1, 3, 3])  # Due to padding

    maxpool_node = helper.make_node(
        'MaxPool',
        inputs=['input'],
        outputs=['output'],
        kernel_shape=[2, 2],  # 2x2 pooling
        pads=[1, 1, 1, 1],  # Padding of 1 on all sides
        strides=[2, 2]  # Stride of 2
    )

    graph = helper.make_graph(
        [maxpool_node],
        'MaxPoolModel_WithPadding',
        [input_tensor],
        [output_tensor]
    )

    model = helper.make_model(graph, producer_name='onnx-maxpool-pad-example')
    onnx.save(model, output_filename)
    print(f"Model saved: {output_filename}")

def test_maxpool_model_onnx(model_filename, input_data):
    session = ort.InferenceSession(model_filename)
    outputs = session.run(None, {'input': input_data})
    print(f"ONNX Results:\n{outputs[0]}\n")

def test_maxpool_model_aidge(model_filename, input_data):
    model = aidge_onnx.load_onnx("maxpool_with_padding.onnx")
    tensor= aidge_core.Tensor(input_data)
    model.compile("cpu", aidge_core.dtype.float32, dims=[input_data.shape])

    # Create SCHEDULER
    scheduler = aidge_core.SequentialScheduler(model)

    # Run inference !
    scheduler.forward(data=[tensor])

    for outNode in model.get_output_nodes():
        output_aidge = np.array(outNode.get_operator().get_output(0))
        print(f"Aidge Results:\n{output_aidge}\n")

def test_maxpool_model_torch(input_data):
    tensor = torch.from_numpy(input_data)
    pooled = F.max_pool2d(tensor, kernel_size=2, stride=2, padding=1)
    print(f"Torch Results:\n{pooled.squeeze()}\n")

if __name__ == "__main__":
    create_maxpool_model("maxpool_with_padding.onnx")

    input_data = np.array([[
        [  # Single channel 4x4 input
            [-1, 2, 3, 4],
            [5, 6, 7, -8],
            [9, 10, 11, 12],
            [13, 14, 15, -16]
        ]
    ]], dtype=np.float32)
    print("\nTesting model...\n")
    test_maxpool_model_onnx("maxpool_with_padding.onnx", input_data)
    test_maxpool_model_aidge("maxpool_with_padding.onnx", input_data)
    test_maxpool_model_torch(input_data)

The results: ONNX Results:
[[[[ -1. 3. 4.]
[ 9. 11. 12.]
[ 13. 15. -16.]]]]

Aidge Results:
[[[[ 0. 3. 4.]
[ 9. 11. 12.]
[13. 15. 0.]]]]

Torch Results:
tensor([[ -1., 3., 4.],
[ 9., 11., 12.],
[ 13., 15., -16.]])

changed the description

added Bug 🐛 Important ❗ PriorityHigh labels

changed milestone to %aidge v0.7.0

Idea after talking with @pineapple.

Introduce an argument in Conv, MaxPool and AvgPool ignorePad that would be a list.

This attribute if empty would do nothing. Else, it would contains the size of the padding to ignore.

This will not very efficient would allow the MetaOperator to have the same behavior as the ONNX Operator. And we would still be able to set a specific Implementation to improve performances.

@olivierbichler We think it is an important issue and we need to talk about it!

But I don't see the need to use MetaOperator for paddedPooling. It would be simpler and faster to handle padding within pooling's kernels.
Besides, it would align with backend_cuda where cudnn's pooling operations handle already the padding.

But I don't see the need to use MetaOperator for paddedPooling. It would be simpler and faster to handle padding within pooling's kernels.

The idea is to keep the same behavior were padding and the apdded operator are separated!

Besides, it would align with backend_cuda where cudnn's pooling operations handle already the padding.

For this case, you can create a MetaOperator implementation But here I would like that we solve this issue by having the right behavior with the Pad and Max/AvgPool operator..

We could approach this issue by simply using a default padding in PaddedMaxPooling that is conformant with ONNX and PyTorch behavior. That is, not a padding with 0, but rather a padding that just reflect or duplicate border values so that the max value remains the same. Or, simply padding to the lowest possible value, especially for integers!

In ONNX, average_pooling has a count_include_pad attribute for taking into account or not the padding in the average computation. This seems difficult to achieve with a padding to average value... rather, I would suggest to pad to a specific value that would be ignored by the AvgPooling operator. A good candidate would be NaN for floats.

I like this idea! If we implement those behaviors we need to document them!

Ok I can do that, so max value for ints and nan for float/double?

mentioned in issue aidge_onnx#46