GridSample with bicubic interpolation and border padding

[broune]

I believe that the GridSample op (which recently became an official ONNX op) implementation has a bug on GPU and CPU on line 150 and 210 respectively:

GPU:

onnxruntime/onnxruntime/contrib_ops/cuda/grid_sample_impl.cu

Line 150 in 19b82b4

grid_x_imgSpace = max(0.0f, min(grid_x_imgSpace, W_in - 1.0f));

CPU:

onnxruntime/onnxruntime/contrib_ops/cpu/grid_sample.cc

Line 210 in 19b82b4

x = std::clamp(x, static_cast<T>(0), static_cast<T>(W_in - 1));

This is moving out-of-bounds points to the border when using the border padding mode. This is correct for the nearest interpolation mode and, for complicated reasons, it is also correct for bilinear interpolation. However, bicubic interpolation looks at a 4x4 neighborhood around the data point, so for points that are outside the border, but not far outside, it makes a difference to move the point to the border. At the border, there will be no fractional part in that dimension, so "vision" from the point is limited to the border itself and interior points are irrelevant. However, if the point is slightly outside the border, then there is a fractional part and "vision" from that point extends by 2 units into the in-bounds area, which is enough to pick up an interior point and thus an interior point's value can make a difference. Hence moving a point to the border can remove the contribution of interior points to the interpolated value, creating a discrepancy.

As far as I can see, it is correct to remove this logic, as the border padding mode is additionally handled in a different place. My suspicion, however, is that this logic was added for the purpose to address concern number 8 from this bug report I just made against the ONNX spec, about what happens when the grid point coordinates are very large: onnx/onnx#4033 So maybe there is a reason it would need to stay in some form.

To reproduce, any example with a query point slightly outside the border with the border padding mode and bicubic interpolation should trigger it. The input can be 2x2 with distinct values, e.g. {{1, 2}, {3, 4}} and the query point might be (0.4, 1.1). (I didn't try those specific values myself, let me know if that works).

[skottmckay]

@XiyinOSS could you please take a look?

[XiyinOSS]

Thanks for the finding @broune. A PR will come soon.