webmachinelearning / webnn-polyfill

🧠⚙️ Web Neural Network API polyfill based on TensorFlow.js
https://www.npmjs.com/package/@webmachinelearning/webnn-polyfill
Apache License 2.0
101 stars 18 forks source link

Options validation for autoPad="explict" should be ignored in conv2d #146

Closed Honry closed 2 years ago

Honry commented 2 years ago

Following options validation at https://github.com/webmachinelearning/webnn-polyfill/blob/master/src/nn/ops/conv2d.ts#L43 should be ignored, since the options.padding has a default value [0, 0, 0, 0].

    utils.assert(
        !(options.autoPad === MLAutoPad.explicit &&
          options.padding === undefined),
        'The padding parameter should be assigned when autoPad is explicit.');
Honry commented 2 years ago

@BruceDai, PTAL, thanks!

BruceDai commented 2 years ago

@Honry According to the Spec, options.padding doesn't have default value, it would be undefined. Spec just explains

If not present, the values are assumed to be [0,0,0,0].

The validation is correct for

autoPad: an MLAutoPad. The automatic input padding options. By default, this argument is set to "explicit", which means that the values in the options.padding array should be used for input padding.
Honry commented 2 years ago

The spec doesn't specify that there should be an error thrown when the "options.padding" is undefined and the autoPad is 'explicit'.

My understanding is, according to spec, when autoPad is explicit, the values of options.paddings should be used, i.e. if it's not presented, it will use [0, 0, 0, 0].

BruceDai commented 2 years ago

My understanding is, according to spec, when autoPad is explicit, the values of options.paddings should be used

Correct.

I added this check according to https://github.com/webmachinelearning/webnn-polyfill/pull/93#discussion_r672888590.

Does it need update spec by assigning padding with default [0, 0, 0, 0], @huningxin?

dictionary MLConv2dOptions {
  sequence<long> padding = [0, 0, 0, 0];
  // others options... 
}
huningxin commented 2 years ago

Does it need update spec by assigning padding with default [0, 0, 0, 0], @huningxin?

spec does say that , please check https://webmachinelearning.github.io/webnn/#api-mlgraphbuilder-conv2d

padding: a sequence of long of length 4. The additional rows and columns added to the beginning and ending of each spatial dimension of input, [beginning_height, ending_height, beginning_width, ending_width]. If not present, the values are assumed to be [0,0,0,0].