Repository navigation
Need clarify scale factor for resample2d #610
Description
Link to resample2d definitions: https://webmachinelearning.github.io/webnn/#mlgraphbuilder-calculate-resample-output-sizes.
-
The output sizes by calculating with scales are not correct, and the
desc's shapeis not accurate since the shape is defined as "An MLOperand's shape is its [[descriptor]].dimensions.". - I submitted a pr Bugfix: Updated the step of calculating output sizes by scales for resample2d #611 to fix it, please take a review, thanks. -
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.
Reacted by Joshua Bell, Ningxin Hu and Dwayne Robinson
Activity
inexorabletash commentedon Mar 21, 2024
1 seems addressed by the PR / 24edf7f
The spec doesn't generally handle overflow right now. A more general issue and a convention for handling that would be good.
inexorabletash commentedon Mar 21, 2024
Re: overflow - see #582 (comment) for some discussion
BruceDai commentedon Mar 27, 2024
And the scale value should be positive value which was raised at CL-5382118 reviewing.
@huningxin
huningxin commentedon Mar 27, 2024
The "check resample options" step check the scale value should be positive:
[-]The step "calculate resample output sizes" for resample2d has an issue and need some clarifications for scale factor [/-][+]Need clarify scale factor for resample2d [/+]inexorabletash commentedon Apr 2, 2024
Side observation: the Chromium code has
if (scales[0] < 0 || scales[1] < 0)- it seems like those tests should be<=insteadhttps://source.chromium.org/chromium/chromium/src/+/main:components/ml/webnn/graph_validation_utils.cc;l=1024
huningxin commentedon Apr 3, 2024
+1
@mingmingtasd, could you please take a look at the chromium impl?
mingmingtasd commentedon Apr 3, 2024
Thanks! Good catch! I open: https://issues.chromium.org/issues/332587370 to track, and will fix that.
inexorabletash commentedon Apr 5, 2024
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?
huningxin commentedon Apr 6, 2024
@inexorabletash
I suppose this is not an implementation imposed limit. According to calculate resample output sizes step
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 #391.
inexorabletash commentedon Apr 8, 2024
Heh, thanks for reading the spec for me. :) So we should spec that. I'll put up a PR.
Introduce "valid dimension", used as needed when calculating operand …