-
Notifications
You must be signed in to change notification settings - Fork 3k
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
[js/webgpu] Float16Array polyfill for uniform #19307
Changes from 7 commits
5a95268
706edd4
5453a7a
a8d4d77
ce46073
d6c86a3
3b49dee
da77265
4396346
1b853ba
cb3ba6c
1480c48
d63d39a
82485df
f44bd9c
6d05812
0d9e517
453cfe9
d281faf
2fc4f58
6db94a6
4ae19d2
e9a6cfa
453b52e
7e75c14
861c472
0082c71
b6f4cd5
cecada7
c9e90b1
10e9d65
7ab6a86
3619729
c8c74d2
4cb32ba
3020722
894c408
aa474a8
6795713
432f311
44ee23f
a239ce2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,7 +5,7 @@ import {tensorToDataURL, tensorToImageData} from './tensor-conversion-impl.js'; | |
import {TensorToDataUrlOptions, TensorToImageDataOptions} from './tensor-conversion.js'; | ||
import {tensorFromGpuBuffer, tensorFromImage, tensorFromPinnedBuffer, tensorFromTexture} from './tensor-factory-impl.js'; | ||
import {CpuPinnedConstructorParameters, GpuBufferConstructorParameters, TensorFromGpuBufferOptions, TensorFromImageBitmapOptions, TensorFromImageDataOptions, TensorFromImageElementOptions, TensorFromTextureOptions, TensorFromUrlOptions, TextureConstructorParameters} from './tensor-factory.js'; | ||
import {checkBigInt, NUMERIC_TENSOR_TYPE_TO_TYPEDARRAY_MAP, NUMERIC_TENSOR_TYPEDARRAY_TO_TYPE_MAP, SupportedTypedArray, SupportedTypedArrayConstructors} from './tensor-impl-type-mapping.js'; | ||
import {checkTypedArray, NUMERIC_TENSOR_TYPE_TO_TYPEDARRAY_MAP, NUMERIC_TENSOR_TYPEDARRAY_TO_TYPE_MAP, SupportedTypedArray, SupportedTypedArrayConstructors} from './tensor-impl-type-mapping.js'; | ||
import {calculateSize, tensorReshape} from './tensor-utils-impl.js'; | ||
import {Tensor as TensorInterface} from './tensor.js'; | ||
|
||
|
@@ -67,8 +67,8 @@ export class Tensor implements TensorInterface { | |
arg0: TensorType|TensorDataType|readonly string[]|readonly boolean[]|CpuPinnedConstructorParameters| | ||
TextureConstructorParameters|GpuBufferConstructorParameters, | ||
arg1?: TensorDataType|readonly number[]|readonly string[]|readonly boolean[], arg2?: readonly number[]) { | ||
// perform one-time check for BigInt support | ||
checkBigInt(); | ||
// perform one-time check for BigInt/Float16Array support | ||
checkTypedArray(); | ||
|
||
let type: TensorType; | ||
let dims: readonly number[]; | ||
|
@@ -146,8 +146,8 @@ export class Tensor implements TensorInterface { | |
// Throw error here because when user try to use number array as data, | ||
// e.g. new Tensor('float16', [1, 2, 3, 4], dims)), it will actually call | ||
// Uint16Array.from(arg1) which generates wrong data. | ||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. as explained in the old error message, you cannot do There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done, thanks! |
||
} else if (arg0 === 'uint64' || arg0 === 'int64') { | ||
// use 'as any' here because: | ||
// 1. TypeScript's check on type of 'Array.isArray()' does not work with readonly arrays. | ||
|
@@ -168,6 +168,8 @@ export class Tensor implements TensorInterface { | |
} | ||
} 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, this is not necessary now. Thanks! |
||
} else { | ||
throw new TypeError(`A ${type} tensor's data must be type of ${typedArrayConstructor}`); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,5 +30,8 @@ | |
"ONNXRuntime", | ||
"ONNX Runtime" | ||
], | ||
"description": "ONNXRuntime JavaScript API library" | ||
"description": "ONNXRuntime JavaScript API library", | ||
"dependencies": { | ||
"@petamoriken/float16": "^3.8.4" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please add as devDependencies There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
// Copyright (c) Microsoft Corporation. All rights reserved. | ||
// Licensed under the MIT License. | ||
|
||
import {Float16Array} from '@petamoriken/float16'; | ||
import assert from 'assert/strict'; | ||
import {Tensor} from 'onnxruntime-common'; | ||
|
||
|
@@ -34,7 +35,7 @@ export const BIGINT_TYPES = [ | |
/** | ||
* 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
|
||
/** | ||
* A list of all numerical types. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -241,8 +241,8 @@ const prepareInputs = (inputs: Tensor[], attributes: UpsampleAttributes): [reado | |
|
||
const parseScalesData = (scale: Tensor, mode: string, isResize: boolean): number[] => { | ||
const scales = Array.from(scale.floatData); | ||
scalesValidation(scales, mode, isResize); | ||
return scales; | ||
scalesValidation(scales as number[], mode, isResize); | ||
return scales as number[]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove this may complains: $ npm run build
lib/onnxjs/backends/webgl/ops/resize-packed.ts:244:20 - error TS2345: Argument of type 'unknown[]' is not assignable to parameter of 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 . There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. #19358 should work with it |
||
}; | ||
|
||
const parseScalesDataFromOutputSize = | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,6 +11,7 @@ | |
"version": "1.18.0", | ||
"jsdelivr": "dist/ort.min.js", | ||
"dependencies": { | ||
"@petamoriken/float16": "^3.8.4", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please add as devDependencies There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
"flatbuffers": "^1.12.0", | ||
"guid-typescript": "^1.0.9", | ||
"long": "^5.2.3", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
[ | ||
{ | ||
"name": "constant 2D", | ||
"operator": "Pad", | ||
"opset": { "domain": "", "version": 10 }, | ||
"attributes": [ | ||
{ "name": "mode", "data": "constant", "type": "string" }, | ||
{ "name": "value", "data": 1.2, "type": "float" }, | ||
{ "name": "pads", "data": [3, 2, 2, 3], "type": "ints" } | ||
], | ||
"cases": [ | ||
{ | ||
"name": "[2,2]->[7,7]", | ||
"inputs": [ | ||
{ | ||
"data": [1.0, 2.0, 3.0, 4.0], | ||
"dims": [2, 2], | ||
"type": "float16" | ||
} | ||
], | ||
"outputs": [ | ||
{ | ||
"data": [ | ||
1.2, 1.2, 1.2, 1.2, 1.2, 1.2, 1.2, 1.2, 1.2, 1.2, 1.2, 1.2, 1.2, 1.2, 1.2, 1.2, 1.2, 1.2, 1.2, 1.2, 1.2, | ||
1.2, 1.2, 1.0, 2.0, 1.2, 1.2, 1.2, 1.2, 1.2, 3.0, 4.0, 1.2, 1.2, 1.2, 1.2, 1.2, 1.2, 1.2, 1.2, 1.2, 1.2, | ||
1.2, 1.2, 1.2, 1.2, 1.2, 1.2, 1.2 | ||
], | ||
"dims": [7, 7], | ||
"type": "float16" | ||
} | ||
] | ||
} | ||
] | ||
} | ||
] |
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.
isFloat16Array
is not going to be polyfilled in the global scope. we can still usea instanceof Float16Array
to perform the check.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.
done