Skip to content
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

Clarify the usage of 32 bit floating point type and consider using double #325

Closed
huningxin opened this issue Jan 10, 2023 · 3 comments · Fixed by #647
Closed

Clarify the usage of 32 bit floating point type and consider using double #325

huningxin opened this issue Jan 10, 2023 · 3 comments · Fixed by #647

Comments

@huningxin
Copy link
Contributor

This issue was raised during Chromium CL review. Thanks Jiewei @wacky6 for the valuable feedback!

The WebIDL 32 bit floating point type float is widely used by WebNN spec, such as for setting min and max value of clamp operator:

dictionary MLClampOptions {
  float minValue;
  float maxValue;
};

However, WebIDL spec has a warning of using float and the recommendation is to use double.

Unless there are specific reasons to use a 32 bit floating point type, specifications should use double rather than float, since the set of values that a double can represent more closely matches an ECMAScript Number.

According to that, Jiewei suggested that the spec should mention reason of using float (at least an informative note) and propose using double instead. The reasons include (pasted from the CL thread):

  • Don't see particular reasons for limiting this to fp32
  • What if (in the future) we have backends that can work with fp64? In this case fp32 becomes a limitation.
  • Would it be worthwhile specifying float point casting behavior in the spec? Say, if caller passes in a fp64 number (that is too large to represent in fp32/fp16), how should backend interpret this (cast to infinity, error, find closest value non-infinity, something else)?
@wchao1115
Copy link
Collaborator

FWIW, double is not a considered a mainstream native data type on the GPU. Although there is a shader model extension for it, it's not until recently (e.g. HLSL shader model 5)

@wchao1115
Copy link
Collaborator

wchao1115 commented Apr 27, 2023

Just an additional note that extending float attributes e.g. minValue, maxValue in the options dictionary should be straightforward. If it makes the spec more in line with the rest of WebIDL specs, I think it's reasonable. However, I have some reservations about adding support for float64 type as an operand type. I don't think it's needed. For instance, on the GPU, float64 tensor type is not widely supported. New models only tend to become smaller with lower precision and the need for a double-precision trained parameter is unheard of.

inexorabletash added a commit to inexorabletash/webnn that referenced this issue Apr 18, 2024
For some MLGraphBuilder methods the type of a numeric input can vary -
e.g. for constant() an explicit MLOperandDataType is provided; for
clamp() and pad() the data type is implied by input operands. In these
cases, specifying the numeric value as either a float/double or int64
type runs into accuracy or range issues - you can't accurately
represent all int64 values as a double, and you can't represent the
full range of floats as int64. (You also can't represent all int64
values as an long long either - over 2^53 things get wierd. But that's
a digression.)

Per discussion in whatwg/webidl#1388 this
change introduces a union between a bigint type and unrestricted
double called MLNumber. Callers can pass a JS number (1234, 1.1234e38)
or a JS bigint (9007199254740993n), and the implementation will treat
it properly based on the explicit or implicit MLOperandDataType. Usage
of this type should be limited to only those cases.

Fixes webmachinelearning#442

Note that webmachinelearning#492 proposes changes to the constant sequential filling
operation; this just adds IDL to match the current spec prose.

Some of the concerns raised in webmachinelearning#325 are addressed (e.g. clamp()'s
options). However, several other options are still specified as
"float", and should maybe be "double" - but MLNumber is likely not
appropriate for those, so they are not updated here.
@inexorabletash
Copy link
Member

Discussion here and elsewhere hasn't explicitly mentioned the resample2d() scales option, which is defined as sequence<float>. This isn't part of graph execution, but a convenience for calculating the integer target sizes. #474 proposes remove the option as a simplification, but if kept it should probably be changed to double.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants