-
Notifications
You must be signed in to change notification settings - Fork 2.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Dubious implementation of ROIAlign in 'max' mode #6146
Comments
Would you like to contribute? |
@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 |
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 ? |
@yury-intel : I found a bug in This:
Should be:
|
Yes I remember this was a bug in ORT |
i found same issue, does onnxruntime will fix it on future? |
@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... |
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
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.
The text was updated successfully, but these errors were encountered: