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

Remove the support of negative dimension values #279

Closed
huningxin opened this issue Aug 2, 2022 · 7 comments · Fixed by #281
Closed

Remove the support of negative dimension values #279

huningxin opened this issue Aug 2, 2022 · 7 comments · Fixed by #281

Comments

@huningxin
Copy link
Contributor

huningxin commented Aug 2, 2022

The existing spec allows to use negative value to define unknown dimension in an MLOperandDescriptor as

dictionary MLOperandDescriptor {
  // The operand type.
  required MLOperandType type;

  // The dimensions field is only required for tensor operands.
  // The negative value means an unknown dimension.
  sequence<long> dimensions;
};

However, according to the investigation of native ML APIs used by WebNN-native implementation, this feature is not supported widely. The following table captures some examples for reference:

native API data type of dimension value
XNNPACK size_t
DirectML UINT
NNAPI uint32_t
MLAS size_t
OpenVINO size_t
BNNS size_t
MPS NSUInteger

Given that, I'd like to propose to remove the support of negative dimension values and use unsigned long data type instead.

/cc @yuhonglin, thanks for raising this issue when reviewing a Chromium WebNN CL.

@gramalingam
Copy link
Contributor

The use of negative indices is to enable dynamic-tensors, or tensors whose sizes are not known at the graph-build time and will be known only when running the model-inference. They are used only when a static tensor shape descriptor is required for a dynamic tensor. (There is a distinction between a static tensor shape descriptor and a (runtime) tensor shape descriptor.)

Is the goal to disallow dynamic tensors? Or, is it something different?

@huningxin
Copy link
Contributor Author

Is the goal to disallow dynamic tensors?

Yes. From implementation perspective, this feature is not supported by major native ML APIs (listed in above table).

The support of dynamic-tensors would be left to apps or frameworks. The WebNN graphs could be built and computed when the dynamic-tensors' sizes are known during model-inference time.

WDYT?

@wacky6
Copy link

wacky6 commented Aug 4, 2022

Tensorflow (without using XLA) has some support for dynamic size (e.g. a model can accept an image of arbitrary size). The unknown dimension will take the shape of the first input.

Changing the shape will retrace the computation graph (i.e. rebuild the graph), and will negatively impact performance.

XLA and TPU backend don't support dynamic size input IIUC, and all dimensions needs to be known at graph creation time.

I think it's fairly reasonable to require explicit dimensions in WebNN and let JS libraries or applications to deal with dynamic shapes (e.g. resize the input, rebuild the graph, ...)

If WebNN spec allows dynamic shapes, we should probably add an informative paragraph explaining the caveats (e.g. higher latency for 1st inference and shape change) considering backends probably don't support them natively.

@anssiko
Copy link
Member

anssiko commented Aug 18, 2022

Given this issue substantially impacts implementations of the API features in CR scope, I'm marking this as "cr".

@anssiko anssiko added the cr label Aug 18, 2022
@wchao1115
Copy link
Collaborator

Given that WebNN is intended to be a backend API, shape inference should really belong to the frontend framework. I support the idea of enforcing it so in the API contract. @huningxin Is this the only place in the API that indicates dynamic tensor support? Do we define support for dynamic shape inference anywhere else in any op?

@huningxin
Copy link
Contributor Author

@wchao1115

Is this the only place in the API that indicates dynamic tensor support? Do we define support for dynamic shape inference anywhere else in any op?

I understand the API only allows to define dynamic tensor for input operand. I am not aware any ops support dynamic shape inference.

@huningxin
Copy link
Contributor Author

@RafaelCintron , as we discussed in WG call – 25 August 2022, the info of macOS APIs (BNNS and MPS) has been added into the table, they all support unsigned integer type.

/cc @anssiko

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

Successfully merging a pull request may close this issue.

5 participants