[js/webgpu] Float16Array polyfill for uniform#19307
[js/webgpu] Float16Array polyfill for uniform#19307axinging wants to merge 42 commits intomicrosoft:mainfrom
Conversation
| declare global { | ||
| // eslint-disable-next-line @typescript-eslint/naming-convention, @typescript-eslint/no-explicit-any | ||
| const Float16Array: any; | ||
| const isFloat16Array: any; |
There was a problem hiding this comment.
isFloat16Array is not going to be polyfilled in the global scope. we can still use a instanceof Float16Array to perform the check.
js/common/lib/tensor-impl.ts
Outdated
| throw new TypeError( | ||
| 'Creating a float16 tensor from number array is not supported. Please use Uint16Array as data.'); | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| data = (typedArrayConstructor as any).from(arg1); |
There was a problem hiding this comment.
as explained in the old error message, you cannot do typedArrayConstructor.from because it does not work with Uint16Array. you need to check if typedArrayConstructor is Float16Array. You can just use the code from that of my PR.
js/common/lib/tensor-impl.ts
Outdated
| } else if (arg1 instanceof typedArrayConstructor) { | ||
| data = arg1; | ||
| } else if (isFloat16Array !== undefined && isFloat16Array(arg1)) { | ||
| data = arg1 as InstanceType<typeof Float16Array>; |
There was a problem hiding this comment.
does the code actually call into this if branch? I assume that map NUMERIC_TENSOR_TYPE_TO_TYPEDARRAY_MAP should already set with key 'float16', so it should be handled in condition (arg1 instanceof typedArrayConstructor)
There was a problem hiding this comment.
No, this is not necessary now. Thanks!
| scalesValidation(scales as number[], mode, isResize); | ||
| return scales as number[]; |
There was a problem hiding this comment.
we can keep onnxjs code unchanged as they are going to deprecated. we don't plan to support new features to webgl.
There was a problem hiding this comment.
Remove this may complains: $ npm run build
[email protected] prebuild
tsc -p . --noEmit && tsc -p lib/wasm/proxy-worker --noEmit
lib/onnxjs/backends/webgl/ops/resize-packed.ts:244:20 - error TS2345: Argument of type 'unknown[]' is not assignable to parameter of type 'number[]'.
Type 'unknown' is not assignable to type 'number'.
244 scalesValidation(scales, mode, isResize);
~~~~~~
lib/onnxjs/backends/webgl/ops/resize-packed.ts:245:3 - error TS2322: Type 'unknown[]' is not assignable to type 'number[]'.
245 return scales;
~~~~~~
Found 2 errors in the same file, starting at: lib/onnxjs/backends/webgl/ops/resize-packed.ts:244
There was a problem hiding this comment.
let me try to make some modifications to unit test so that it no longer depends on onnxjs/tensor. Then you don't need to modify any file under onnxjs folder .
js/common/package.json
Outdated
| "description": "ONNXRuntime JavaScript API library" | ||
| "description": "ONNXRuntime JavaScript API library", | ||
| "dependencies": { | ||
| "@petamoriken/float16": "^3.8.4" |
There was a problem hiding this comment.
please add as devDependencies
js/web/package.json
Outdated
| "version": "1.18.0", | ||
| "jsdelivr": "dist/ort.min.js", | ||
| "dependencies": { | ||
| "@petamoriken/float16": "^3.8.4", |
There was a problem hiding this comment.
please add as devDependencies
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| import {Float16Array} from '@petamoriken/float16'; |
There was a problem hiding this comment.
no need to add this as we don't plan to add f16 support for webgl
js/common/test/unit-tests/common.ts
Outdated
| * float16 type, data represented by Uint16Array | ||
| */ | ||
| export const FLOAT16_TYPE = ['float16', Uint16Array, false] as const; | ||
| export const FLOAT16_TYPE = ['float16', Float16Array, false] as const; |
There was a problem hiding this comment.
I suggest not to update this part. Both paths (with and without polyfill) need to be tested. to test f16 polyfill, may need to add a separated target.
| declare global { | ||
| // eslint-disable-next-line @typescript-eslint/naming-convention, @typescript-eslint/no-explicit-any | ||
| const Float16Array: any; | ||
| const isFloat16Array: any; |
js/common/package.json
Outdated
| "description": "ONNXRuntime JavaScript API library" | ||
| "description": "ONNXRuntime JavaScript API library", | ||
| "dependencies": { | ||
| "@petamoriken/float16": "^3.8.4" |
js/web/package.json
Outdated
| "version": "1.18.0", | ||
| "jsdelivr": "dist/ort.min.js", | ||
| "dependencies": { | ||
| "@petamoriken/float16": "^3.8.4", |
js/common/lib/tensor-impl.ts
Outdated
| throw new TypeError( | ||
| 'Creating a float16 tensor from number array is not supported. Please use Uint16Array as data.'); | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| data = (typedArrayConstructor as any).from(arg1); |
js/common/lib/tensor-impl.ts
Outdated
| } else if (arg1 instanceof typedArrayConstructor) { | ||
| data = arg1; | ||
| } else if (isFloat16Array !== undefined && isFloat16Array(arg1)) { | ||
| data = arg1 as InstanceType<typeof Float16Array>; |
There was a problem hiding this comment.
No, this is not necessary now. Thanks!
| scalesValidation(scales as number[], mode, isResize); | ||
| return scales as number[]; |
There was a problem hiding this comment.
Remove this may complains: $ npm run build
[email protected] prebuild
tsc -p . --noEmit && tsc -p lib/wasm/proxy-worker --noEmit
lib/onnxjs/backends/webgl/ops/resize-packed.ts:244:20 - error TS2345: Argument of type 'unknown[]' is not assignable to parameter of type 'number[]'.
Type 'unknown' is not assignable to type 'number'.
244 scalesValidation(scales, mode, isResize);
~~~~~~
lib/onnxjs/backends/webgl/ops/resize-packed.ts:245:3 - error TS2322: Type 'unknown[]' is not assignable to type 'number[]'.
245 return scales;
~~~~~~
Found 2 errors in the same file, starting at: lib/onnxjs/backends/webgl/ops/resize-packed.ts:244
js/common/test/unit-tests/common.ts
Outdated
| * float16 type, data represented by Uint16Array | ||
| */ | ||
| export const FLOAT16_TYPE = ['float16', Uint16Array, false] as const; | ||
| export const FLOAT16_TYPE = ['float16', Float16Array, false] as const; |
js/web/test/data/ops/pad_f16.jsonc
Outdated
| { | ||
| "name": "constant 2D", | ||
| "operator": "Pad", | ||
| "opset": { "domain": "", "version": 10 }, |
There was a problem hiding this comment.
Can you also add a test for version > 10, in which the value is as an input not attribute and have the type float16? I suppose this can test the float16 uniform?
Do you need to update the code here?
test/test-main.ts:18:12 - error TS2339: Property 'Float16Array' does not exist on type 'typeof globalThis'. 18 globalThis.Float16Array = Float16Array;
|
This is replaced by: |
To use this feature, first create a f16 model from f32 model:
Then test it like this: