Need clarify scale factor for resample2d

[BruceDai]

Link to resample2d definitions: https://webmachinelearning.github.io/webnn/#mlgraphbuilder-calculate-resample-output-sizes.

1) The output sizes by calculating with scales are not correct, and the desc's shape is not accurate since the shape is defined as "An MLOperand's shape is its [[descriptor]].dimensions.". - I submitted a pr #611 to fix it, please take a review, thanks.

2) And it needs clarify the limitations for the product of scale factor and spatial dimension's size, I write such tests in my CL-5382118, please take a look, thanks.

[inexorabletash]

1 seems addressed by the PR / 24edf7f775ccb5105b3503449c714e2b7af563e6

The spec doesn't generally handle overflow right now. A more general issue and a convention for handling that would be good.

[inexorabletash]

Re: overflow - see https://github.com/webmachinelearning/webnn/pull/582#discussion_r1503641101 for some discussion

[BruceDai]

And the scale value should be positive value which was raised at CL-5382118 reviewing.

@huningxin

The current spec doesn't put any restrictions on scale value, however the implementation 1 requests it to be positive value.

[huningxin]

The "check resample options" step check the scale value should be positive:

If options.scales does not exist, set it to the list « 1.0, 1.0 ». Otherwise, if any of its values is not greater than 0, or if its size is not 2, return false.

[inexorabletash]

Side observation: the Chromium code has if (scales[0] < 0 || scales[1] < 0) - it seems like those tests should be <= instead

https://source.chromium.org/chromium/chromium/src/+/main:components/ml/webnn/graph_validation_utils.cc;l=1024

[huningxin]

Side observation: the Chromium code has if (scales[0] < 0 || scales[1] < 0) - it seems like those tests should be <= instead

+1

https://source.chromium.org/chromium/chromium/src/+/main:components/ml/webnn/graph_validation_utils.cc;l=1024

@mingmingtasd, could you please take a look at the chromium impl?

[mingmingtasd]

Side observation: the Chromium code has if (scales[0] < 0 || scales[1] < 0) - it seems like those tests should be <= instead

https://source.chromium.org/chromium/chromium/src/+/main:components/ml/webnn/graph_validation_utils.cc;l=1024

Thanks! Good catch! I open: https://issues.chromium.org/issues/332587370 to track, and will fix that.

[inexorabletash]

Re: clarify the limitations for the product of scale factor and spatial dimension's size

I filed #636 for the general issue. One thing I'm not clear on reading the code (https://source.chromium.org/chromium/chromium/src/+/main:components/ml/webnn/graph_validation_utils.cc;l=974 I think) is: is this just an implementation imposed limit because the Chromium implementation happens to use uint32_t internally?

• If it's just an implementation-imposed limit, I think we should just reference #636 and close this issue
• Otherwise, we should add something to the spec specific to resample2d

[huningxin]

@inexorabletash

One thing I'm not clear on reading the code (https://source.chromium.org/chromium/chromium/src/+/main:components/ml/webnn/graph_validation_utils.cc;l=974 I think) is: is this just an implementation imposed limit because the Chromium implementation happens to use uint32_t internally?

I suppose this is not an implementation imposed limit. According to calculate resample output sizes step

Otherwise, set desc.dimensions[options.axes[index]] to floor(input’s shape[options.axes[index]] * options.scales[index]).

Because desc.dimensions[options.axes[index]] is of type uint32, if the result of floor(input’s shape[options.axes[index]] * options.scales[index]) is out of range of uint32, it should throw a TypeError. I guess the value 0 limit is due to WebNN doesn't allow 0 size dimension that is being discussed at https://github.com/webmachinelearning/webnn/issues/391.

[inexorabletash]

Because desc.dimensions[options.axes[index]] is of type uint32, if the result of floor(input’s shape[options.axes[index]] * options.scales[index]) is out of range of uint32, it should throw a TypeError.

Heh, thanks for reading the spec for me. :) So we should spec that. I'll put up a PR.