Skip to content
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

[webnn] Add float32 tests for WebNN resample2d op #41811

Merged
merged 4 commits into from
Mar 14, 2024

Conversation

BruceDai
Copy link
Contributor

@BruceDai BruceDai commented Sep 5, 2023

@fdwr This PR is to add 13 float32 resample2d tests. I ran these tests by internal Chromium build using WebNN DirectML backend, got 10 PASS and 3 FAIL.
image

These 3 FAIL tests are all for resample(down sampling) on "nearest-neighbor" mode with below 4D input of shape [1, 1, 4, 6]

// nchw layout
[[[[ 3.86005284, 45.18463077, 87.67153743, 98.78210347, 66.37414347,  3.41158349],
   [86.14930501, 95.9813372 , 76.87126314, 16.52591355, 65.98782867, 25.47092156],
   [22.56010548, 92.08479613, 85.80876635, 92.63166027, 29.91620871, 75.40461275],
   [62.06375451,  1.77121588, 99.47231285, 11.44055014, 25.39634271, 67.02175102]]]]

by options.sizes = [2, 3] or options.scales = [0.5, 0.5],

output is of shape [1, 1, 2, 3] and its actual value as below

// using bottom right data
[[[[95.9813372 , 16.52591355, 25.47092156],
   [ 1.77121588, 11.44055014, 67.02175102]]]]

while our expected data were generated by https://github.com/webmachinelearning/webnn-baseline/blob/main/src/resample2d.js with same input data and options:

// using top left data
[[[[ 3.86005284, 87.67153743, 66.37414347],
   [22.56010548, 85.80876635, 29.916208715]]]]

I'm not sure which is wrong on

  1. Pure JavaScripts reampled2d implementation for WebNN Baseline
  2. Chromium implementation for reampled2d on DirectML backend

Do you have any ideas? Thanks.

@fdwr
Copy link

fdwr commented Sep 9, 2023

I'm not sure which is wrong on

Both are reasonable and not really "wrong" - we just have to pick one. The only truly wrong ones are those libraries that always floor pixels (broken libraries) which result in shifted images, but I believe most image resampling API's round input coordinate on halves toward negative infinity, and ONNX's Resize defaults nearest_mode to round_prefer_floor.

This is easy to control via DML_RESAMPLE2_OPERATOR_DESC in Feature Level 5.1 with the RoundingDirection (used here in the ORT DML EP), but I'm concerned how to achieve it with the older RESAMPLE1 🤔, as my previous comment in the ORT DML EP said that "round_prefer_floor" could not be achieved without an API update. The older function used floor(input.x + 0.5), but that's effectively rounding halves up! ⏳...

@BruceDai
Copy link
Contributor Author

@fdwr May we first to land PASS tests and create another pr of these failure 3 tests on "nearest-neighbor" mode which would help for resolving https://issues.chromium.org/issues/327337526, thanks.

@fdwr
Copy link

fdwr commented Mar 11, 2024

@fdwr May we first to land PASS tests and create another pr of these failure 3 tests on "nearest-neighbor" mode which would help for resolving https://issues.chromium.org/issues/327337526, thanks.

Yeah, go ahead 👍.

@BruceDai
Copy link
Contributor Author

Thanks @fdwr. I've removed failure tests an rebased code, please take another look. Thanks a lot.

Link to split tests on #45017

* Get ULP tolerance of resample2d operations.
* @param {Object} resources - Resources used for building a graph
* @param {String} operationName - An operation name
* @returns {Number} A tolerance number
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @returns {Number} A tolerance number
* @returns {Number} A tolerance number in ULP

(to clarify it's not absolute difference, because 84 would be a huuuge tolerance 😉)

tolerance = 84;
} else if (precisionType === 'float16') {
tolerance = 10;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

else {
  tolerance = 1;
}

tolerance remains uninitialized in the else code path path. For integer types (e.g. uint8), even if many backends don't currently support them, a reasonable default for linear is 1 ULP (due to rounding up or down).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated. Thanks.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note my comment above too (in case you missed it), but approved either way.

Copy link
Contributor Author

@BruceDai BruceDai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fdwr Please take another look, thanks.

tolerance = 84;
} else if (precisionType === 'float16') {
tolerance = 10;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated. Thanks.

Copy link
Contributor

@Honry Honry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@Honry Honry merged commit efd2510 into web-platform-tests:master Mar 14, 2024
17 checks passed
BruceDai added a commit to BruceDai/wpt that referenced this pull request Mar 25, 2024
…#41811)

* [webnn] Add float32 tests for WebNN resample2d op

* [webnn] Removed some downsample tests for resample2d

* [webnn] Rebased code

* [webnn] Add default ULP tolerance for resample2d on linear mode.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants