[webnn] Add tests for WebNN API constant (fillSequence) #43801
Conversation
| @@ -291,6 +291,7 @@ const PrecisionMetrics = { | |||
| batchNormalization: {ULP: {float32: 6, float16: 6}}, | |||
| clamp: {ULP: {float32: 0, float16: 0}}, | |||
| concat: {ULP: {float32: 0, float16: 0}}, | |||
| constant: {ULP: {float32: 2, float16: 2, int32: 0, uint32: 0, int64: 0, int8: 0, uint8: 0}}, | |||
There was a problem hiding this comment.
This is an interesting case because there are two constant overloads, with one that should be always exact (the one taking the Float32Array), but the other with start and step could have a ULP delta of 2.
Technically the integers could be off by 1 too for large values too (above 16777216) since the constant overload only accepts float start and step values, but none of our test cases go near that high. So in the general case, 0 ULP is valid for integers, and that's fine.
|
@fdwr Thanks for your reviewing! |
| @@ -616,7 +616,7 @@ | |||
| "name": "constant float32 4D tensor of int8 type", | |||
| "inputs": { | |||
| "start": { | |||
| "data": 0.22992068529129028, | |||
| "data": -5.520709991455078, | |||
| "type": "float32" | |||
There was a problem hiding this comment.
Wait, this isn't supported, mixing and matching different data types like float32 and int8. If the output is an integer tensor, then the start and delta must be integers too. It would be legal to pass start of -5 and delta of 1 though. Otherwise DirectML will complain (fail API validation) that the DML_FILL_VALUE_SEQUENCE_OPERATOR_DESC::ValueDataType does not match the outputTensorDesc->Type.
Bin Miao (https://chromium-review.googlesource.com/c/chromium/src/+/5091540) is currently working on a way via the IDL to express both float64 and int64 in the API.
Also by mixing types, you'll get weird artifacts like this repeated 0 x 3 in the output data:
-2,
-1,
0, // <-----
0, // <-----
0, // <-----
1,
2,
There was a problem hiding this comment.
So we need use integer numbers for such cases, though Spec define start and step as float, or does it need update this Spec?
start: a [float](https://webidl.spec.whatwg.org/#idl-float) scalar. The starting value of the range.
step: a [float](https://webidl.spec.whatwg.org/#idl-float) scalar. The gap value between two data points in the range.
There was a problem hiding this comment.
Yes, we should update the spec with additional wording. Even if you pass it as a float/double, internally it will be casted to an integer before any calculations are done with it. So then the baseline should cast any floating-point values (start and delta values) to integers (or BigInt) before computing.
There was a problem hiding this comment.
For now, for the integer cases, can we just pass integer start and delta values?
"data": -5.520709991455078, -> "data": -5,
There was a problem hiding this comment.
Sure, I'll update test data for those integer cases.
There was a problem hiding this comment.
Done, please have another look, thanks.
There was a problem hiding this comment.
One case with a negative step for the integers would be good, but LGTM.
There was a problem hiding this comment.
Added. Please take another look, thanks.
| @@ -616,7 +616,7 @@ | |||
| "name": "constant float32 4D tensor of int8 type", | |||
| "inputs": { | |||
| "start": { | |||
| "data": 0.22992068529129028, | |||
| "data": -5.520709991455078, | |||
| "type": "float32" | |||
There was a problem hiding this comment.
One case with a negative step for the integers would be good, but LGTM.
* [webnn] Add tests for WebNN API constant (fillSequence) * [webnn] Update test data with negative start * [webnn] Update test data for integer test cases * [webnn] Add a test with negative integer step
Current this pr doesn't include tests of uint64 type which will be added later.
@fdwr @Honry PTAL, thanks!