Dubious implementation of ROIAlign in 'max' mode

[yury-intel]

There are cases, when the results of ROIAlign may be significally different from expected.

Implementation of ROIAlign runs questionably in MAX mode.

onnxruntime/onnxruntime/core/providers/cpu/object_detection/roialign.cc

Line 216 in 7f9d955

T val = std::max(

I.e., when weighed data feature_map is collected, we commit MAX - operation to all collected data.
So, we return one of weighed value (by [0, 1] - coefficient), which is assuredly less, then all interpolated values.
In the case of centered coordinates of sampling point, nearly 1/4 from expected value is returned.
We need instead, at first, commit interpolation, and afterwards given type of operation (AVG or MAX).
Suggested algorithm is similar to Caffe implementation.

[pranavsharma]

Would you like to contribute?

[fdwr]

@yury-intel: I found some alignment issues too (comparing against PyTorch, the Mask RCNN paper, and FB research's Detectron 2). Might be the same cause, a half pixel off? #6921

[hariharans29]

Not quite following this. Can you quantify "running questionably" with a simple test case ? For a given input, what do you expect to see vs what actually gets produced ?

[fdwr]

@yury-intel : I found a bug in max mode too after running our WindowsAI ONNX conformance tests, which will probably fix your issue too:
https://github.com/microsoft/onnxruntime/pull/7354/files#diff-5498c6fd67f5974ca5cb7692c44c9d881ce74cc59a8d1109bb9b7bd33941ef3aR212

This:

                  T val = std::max(
                      std::max(std::max(pc.w1 * offset_bottom_data[pc.pos1], pc.w2 * offset_bottom_data[pc.pos2]),
                               pc.w3 * offset_bottom_data[pc.pos3]),
                      pc.w4 * offset_bottom_data[pc.pos4]);

Should be:

                  T val = pc.w1 * offset_bottom_data[pc.pos1] + pc.w2 * offset_bottom_data[pc.pos2] +
                          pc.w3 * offset_bottom_data[pc.pos3] + pc.w4 * offset_bottom_data[pc.pos4];

[masahi]

Yes I remember this was a bug in ORT roi_align with max mode, where summation (for averaging) for bilinear interpolation is incorrrectly replaced with max. Maximization should be done on interpolated values, interpolation itself is always sum (averaging).

[CBlackBird]

Yes I remember this was a bug in ORT roi_align with max mode, where summation (for averaging) for bilinear interpolation is incorrrectly replaced with max. Maximization should be done on interpolated values, interpolation itself is always sum (averaging).

i found same issue, does onnxruntime will fix it on future?

[fdwr]

@hariharans29 I'm unsure who the "owner" of CPU RoiAlign is, but I'll try to push my old PR again from above. Last time some tests failed because they were based on the bad expected results using the old numbers, and I don't know how they were generated in the first place. So I'm unsure what to do there, besides disabling those bad tests 🤔, but I'll sync with master to see what new/old failures surface...