-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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 float16 tests for WebNN concat op #42420
[webnn] Add float16 tests for WebNN concat op #42420
Conversation
webnn/resources/utils.js
Outdated
|
||
/* This method is faster than the OpenEXR implementation (very often | ||
* used, eg. in Ogre), with the additional benefit of rounding, inspired | ||
* by James Tursa?s half-precision code. */ |
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.
Tursa's?
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.
Thanks, fixed.
webnn/resources/utils.js
Outdated
const getTypedArrayData = (type, data) => { | ||
let outData; | ||
if (type === 'float16') { | ||
// workaround to convert Float16 to Unit16 |
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.
Uint16
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.
Thanks, fixed.
actualBitwise = actual[i]; | ||
// convert expected data of Float16 to Uint16 | ||
expectedBitwise = toHalf(expected[i]); | ||
} | ||
distance = actualBitwise - expectedBitwise; | ||
distance = distance >= 0 ? distance : -distance; |
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.
Isn't this just distance = Math.abs(distance)
?
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.
👍
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.
LGTM, thanks!
I'm still working on implement scripts to generate test data & baseline data for float16 tests based on previous test data & baseline data of float32 precision.
Here I firstly submit this PR to show the overview of testing float16 precision.
These new added float16 tests in webnn/resources/test_data/concat.json use the same input data of float64 precision as ones used for testing float32 precision before, and such expected baseline data of float16 precision.
I'm struggled on readability like Solution-1 which is usingwith this PR and simple like Solution-2.
Solution 1 - positive: do not need modify more current test framework / negative: exist many duplicated test data code
Solution 2 - positive: simple, no duplicated test data code / negative: need modify more to support this updated struct, for examples, need iterate precision keys list of "expected" dictionary to test float32 and float16 precisions, according to precision key to prepare relative TypedArray data for input/constant operands and output operand(s), generate each test name with precision word for them to show clearly on UI, etc.,
@fdwr PTAL, and any suggestions, thanks.