search

webmachinelearning / webnn

🧠 Web Neural Network API
https://www.w3.org/TR/webnn/
Other
393 stars 47 forks source link

Use tensor type for the padding rather than MLOperand #224

Closed mingmingtasd closed 2 years ago

mingmingtasd commented 3 years ago

The pad in the spec uses the MLOperand padding as its sencond input. But I find that the pad in DML and in IE, padding was defined with a tensor type. I think it's better to use tensor type to avoid the gap between spec and implementation.

huningxin commented 2 years ago

As feedback of WebML WG Teleconference – 18 Nov 2021, the static attribute is preferred, e.g. @wchao1115 mentioned

+1 on prefer static, dynamism breaks accelerator's pipelining

Another reference is TensorFlow.js, according to tf.pad, the paddings is also an array.

With that, I propose to change the padding input of WebNN's pad to an array (sequence in WebIDL).

@mingmingtasd , I am not sure whether it aligns with your proposal. What do you mean by using "tensor type"?

mingmingtasd commented 2 years ago

As feedback of WebML WG Teleconference – 18 Nov 2021, the static attribute is preferred, e.g. @wchao1115 mentioned

+1 on prefer static, dynamism breaks accelerator's pipelining

Another reference is TensorFlow.js, according to tf.pad, the paddings is also an array.

With that, I propose to change the padding input of WebNN's pad to an array (sequence in WebIDL).

@mingmingtasd , I am not sure whether it aligns with your proposal. What do you mean by using "tensor type"?

Yes, that aligns with what I mean: using array. Thanks! @huningxin

huningxin commented 2 years ago

@mingmingtasd , as @wchao1115 's feedback, DirectML is supporting dynamic padding data. And NNAPI ANEURALNETWORKS_PAD_V2 also supports padding data as an operand. So it looks like the current pad op definition of WebNN is more future-proof.

With that, I would withdraw my proposal and keep the current version. WDYT?

mingmingtasd commented 2 years ago

@mingmingtasd , as @wchao1115 's feedback, DirectML is supporting dynamic padding data. And NNAPI ANEURALNETWORKS_PAD_V2 also supports padding data as an operand. So it looks like the current pad op definition of WebNN is more future-proof.

With that, I would withdraw my proposal and keep the current version. WDYT?

Yes, I agree, thanks for your explaination. @huningxin

huningxin commented 2 years ago

Thanks for the feedback @mingmingtasd ! I'll close PR #233 and this one.