-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Fix #23553: [OP CONFORMANCE][TEMPLATE] Interpolate test fails in op c… #23636
Conversation
…t fails in op conformance
build_jenkins |
hey @runzhangDL please correct formatting to bass clang tests |
Sure, will update soon. Do I need to fix other failed checks? |
I looks like these changes cause below failures - copied from
|
Ok I will try to look into it. It might be easier if I can see the test cases detail to debug why they failed. |
@mitruska Could you please take a look? |
Still working on this, will require a bit more time. It will help if someone can confirm whether we need to add support for batch-last case on interpolation, which might cause failure of other test cases? Or we just remove the interpolation case where batch was put in the last dimension? |
Sorry for the delay, I was distracted by my courseworks, and will look into this issue soon. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @iefode and @runzhangDL, I can see the work is in progress;)
For any changes in the Interpolate reference implementation, please add corresponding unit test to:
std::vector<InterpolateV4TestParams> generateParamsForInterpolate_v4_linear_onnx() { |
build_jenkins |
I am still working on the interpolation implementation, should be done soon |
@mitruska Could you please take a look? |
{ "linear_onnx.resize_downsample_scales_linear_align_corners_channel_last", | ||
Shape{1, 2, 4, 1}, | ||
{1, 2}, | ||
Shape{1, 1, 2, 1}, | ||
{0.6f, 0.6f}, | ||
{1, 2}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test case is okay, but it would be good to add few more tests,
especially with the last dimension not equal to 1
.
hey @runzhangDL, will you find a time to finish this PR? |
This PR will be closed in a week because of 2 weeks of no activity. |
@t-jankowski & @mitruska |
Apology for the late reply. I will try to finish what I left now. I am currently not sure whether my implementation is correct, because the test code didn't work out to me. |
thanks! let us check it |
This PR will be closed in a week because of 2 weeks of no activity. |
build_jenkins |
This PR will be closed in a week because of 2 weeks of no activity. |
This PR was closed because it has been stalled for 2 week with no activity. |
Pull request was closed
The test case failed because it uses a channel-last input pattern, while the linear_onnx_func() assume that all input data have the batch and channel in their first two dimensions. I've added extra condition check in the channel-last case and made it valid to continue the following operations.
Note: The implementation for channel-last input should be different from current one. However, after the input check has been changed, the test case passed already, so I didn't change the following implementation. Please let me know if there are concerns with this and I am happy to add extra adjustment.