| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Could you please share a longer description of this change? Including why this change is made, description of previous
behavior and newly introduced differences etc., See guideline: https://chromium.googlesource.com/chromium/src/+/main/docs/contributing.md#uploading-a-change-for-review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Could you please share a longer description of this change? Including why this change is made, description of previous
behavior and newly introduced differences etc., See guideline: https://chromium.googlesource.com/chromium/src/+/main/docs/contributing.md#uploading-a-change-for-review
+1, very confused why a change which claims to be fixing tolerances for the QDQ subgraph tests is updating the tolerances for a bunch of other tests, but not the QDQ subgraph one.
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
👀
tolerance = sizes.length ?Just for clarity (so others understand what's going on), I'd name it `elementCount`. e.g.
```
const elementCount = sizes.reduce(
(accumulator, currentValue) => accumulator * currentValue, 1);
tolerance = elementCount;
```
reduceL2: tolerance * 2 + 1,I think I wrote +2 last time for the wrapping sqrt.
reduceSum: tolerance * 2,Addition should incur no more loss than multiplication, and reduceProduct takes the elementCount directly. Did you mean to put the `* 2` onto `reduceSumSquare` instead?
Updated. Please take another look, thanks!
Reilly GrantCould you please share a longer description of this change? Including why this change is made, description of previous
behavior and newly introduced differences etc., See guideline: https://chromium.googlesource.com/chromium/src/+/main/docs/contributing.md#uploading-a-change-for-review
+1, very confused why a change which claims to be fixing tolerances for the QDQ subgraph tests is updating the tolerances for a bunch of other tests, but not the QDQ subgraph one.
Could you please share a longer description of this change?
@ningx...@intel.com Added this description, please take another look, thanks!
...is updating the tolerances for a bunch of other tests, but not the QDQ subgraph one
@rei...@chromium.org, The tolerance for the QDQ subgraph tests is defined in `getPrecisionTolerance()` method of utils.js.
And the updated bunch of other tests are for these operators fused in QDQ subgraph, they are using `getPrecisionTolerance()` instead which also provides tolerance for the graph of single operator.
Just for clarity (so others understand what's going on), I'd name it `elementCount`. e.g.
```
const elementCount = sizes.reduce(
(accumulator, currentValue) => accumulator * currentValue, 1);
tolerance = elementCount;
```
👍
I think I wrote +2 last time for the wrapping sqrt.
👍
Addition should incur no more loss than multiplication, and reduceProduct takes the elementCount directly. Did you mean to put the `* 2` onto `reduceSumSquare` instead?
Good catch! 👍
Sorry, I missed put these two tolerance values for `reduceSum` and `reduceSumSquare`, they need swap each other.
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Code-Review | +1 |
// others reduction operators```suggestion
// other reduction operators
```
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Code-Review | +1 |
lgtm with nits
the cumulative tolerance of each operators of subgraph by modifying```suggestion
the cumulative tolerance of each operator of a subgraph by modifying
```
And this CL is also update a bunch of other tests of these operators```suggestion
And this CL also updates a bunch of other tests of operators that are
```
operator, with this CL the updated tolerance definitions is equal to the```suggestion
operator. With this CL, the updated tolerance definitions is equal to the
```
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Code-Review | +1 |
lgtm
the cumulative tolerance of each operators of subgraph by modifying```suggestion
the cumulative tolerance of each operator of a subgraph by modifying
```
Acknowledged
And this CL is also update a bunch of other tests of these operators```suggestion
And this CL also updates a bunch of other tests of operators that are
```
Acknowledged
operator, with this CL the updated tolerance definitions is equal to the```suggestion
operator. With this CL, the updated tolerance definitions is equal to the
```
Acknowledged
```suggestion
// other reduction operators
```
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Thanks Ningxin for helping review latest changes!👍
Current I fail to rebasline expected results after fixing conflict of rebase latest code, I meet "Access blocked: Authorization Error" when executing rebasline command.
Any suggestions? Thanks!
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Thanks Ningxin for helping review latest changes!👍
Current I fail to rebasline expected results after fixing conflict of rebase latest code, I meet "Access blocked: Authorization Error" when executing rebasline command.
Any suggestions? Thanks!
Try running `git cl creds-check` to make sure you have all the necessary credentials set up.
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Code-Review | +1 |
LGTM (once the rebaseline gets added)
Reilly GrantThanks Ningxin for helping review latest changes!👍
Current I fail to rebasline expected results after fixing conflict of rebase latest code, I meet "Access blocked: Authorization Error" when executing rebasline command.
Any suggestions? Thanks!
Try running `git cl creds-check` to make sure you have all the necessary credentials set up.
Thanks Reilly! 👍
I tried this command, and I also switched to another develop machine, the problem still blocks me.
I'm ongoing to find how to fix it.
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
I've updated expected test results. Now all trybots run Pass.
@rei...@chromium.org and @ningx...@intel.com, please take another look, thanks!
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Code-Review | +1 |
LGTM
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Code-Review | +1 |
lgtm
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Auto-Submit | +1 |
| Commit-Queue | +2 |
Thanks all for your review! I'm going to submit this CL.
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
webnn: update tolerance definitions for QDQ subgraph tests
This CL is to update tolerance definitions of QDQ subgraph tests using
the cumulative tolerance of each operator of a subgraph by modifying
`getPrecisionTolerance()` method of `utils.js` help file [1].
And this CL also updates a bunch of other tests of operators that are
fused in QDQ subgraph using `getPrecisionTolerance()` or
`getZeroULPTolerance()` methods instead.
For example, the previous tolerance definitions of this "quantized tanh"
QDQ subgraph test [2] is equal to tolerance definitions of `tanh`
operator. With this CL, the updated tolerance definitions is equal to the
accumulated sum of tolerance of `quantizeLinear` on layer1, tolerance of
`dequantizeLinear` on layer2, tolerance of `tanh` on layer3, tolerance
of `quantizeLinear` on layer4, and tolerance of `dequantizeLinear` on
layer5.
```
input
|
[quantizeLinear] // layer1
|
[dequantizeLinear] // layer2
|
[tanh] // layer3
|
[quantizeLinear] // layer4
|
[dequantizeLinear] // layer5
|
output
```
[1]https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/web_tests/external/wpt/webnn/resources/utils.js;l=39
[2]https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/web_tests/external/wpt/webnn/conformance_tests/qdq_subgraph.https.any.js;l=1761
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
The WPT PR for this CL has been merged upstream! https://github.com/web-platform-tests/wpt/pull/53535
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |