Remove pool2d MLRoundingType - Simplify the operand layout support of conv2d and pooling 2d operations

[fdwr]

For #324

Rather than having two output shape parameters, an explicit output size and an output rounding mode, be more explicit in WebNN such that the caller passes the explicit output shape, resolving any higher-level policy beforehand (like with convTranspose2d):

dictionary MLPool2dOptions : MLOperatorOptions {
  sequence<[EnforceRange] unsigned long> windowDimensions;
  sequence<[EnforceRange] unsigned long> padding;
  sequence<[EnforceRange] unsigned long> strides;
  sequence<[EnforceRange] unsigned long> dilations;
  MLInputOperandLayout layout = "nchw";
- MLRoundingType roundingType = "floor";
  sequence<[EnforceRange] unsigned long> outputSizes;
};

---

Preview | Diff

[huningxin]

outputSizes is optional, we should keep this checking. I feel the existing code has an issue, it should use outputSizes if it is given?

[fdwr]

Yeah, the previous if looked like a typo (so that should be fixed in any case).

Well, with this change, outputSizes would be required (caller is explicit now). Do you think this simplification to be problematic/burdensome? 🤔

[huningxin]

Then outputSizes is not an option anymore, should it be moved to another parameter? Like

 MLOperand averagePool2d(MLOperand input, sequence<[EnforceRange] unsigned long> outputSizes, optional MLPool2dOptions options = {});

[a-sully]

https://w3ctag.github.io/design-principles/#prefer-dictionaries suggests:

You should also consider accepting mandatory parameters through a dictionary, if it would make the API more readable, especially when they are of primitive types.

...but it also says:

The dictionary itself should be an optional argument, so that if the author is happy with all of the default options, they can avoid passing an extra argument.

I'd err on the side of putting required arguments into the dictionary - and making the dictionary itself required - because:

• It's more readable, since primitive types are now described (as the Design Principles suggest)
  • averagePool2d(input, {outputSizes: [2, 3]}) > averagePool2d(input, [2, 3])
• It's more readable, since it's otherwise not obvious to the caller why some configuration options belong in the options dictionary while others don't
  • averagePool2d(input, {outputSizes: [2, 3], layout: "nchw"}) > averagePool2d(input, [2, 3], {layout: "nchw"})
• It's more extensible, since the options dict can be changed without changing the method signature (though making a non-required dictionary field required would still be a breaking change)

[huningxin]

I'd err on the side of putting required arguments into the dictionary - and making the dictionary itself required

SGTM. Thanks for the explanation, @a-sully !

[fdwr]

I'd err on the side of putting required arguments into the dictionary - and making the dictionary itself required

Fine with me. Will do.

and making the dictionary itself required

Hmm, the following doesn't work (bikeshed error as required can be used on fields but evidently not parameters):

  MLOperand averagePool2d(MLOperand input, required MLPool2dOptions options);

So I presume you meant to just remove the "= {}" instead?

  MLOperand averagePool2d(MLOperand input, MLPool2dOptions options);

[huningxin]

If windowDimensions, padding, strides and dilations parameters are not used in the algorithm steps, they probably can be removed.

outputSizes is not optional.

[inexorabletash]

Since this algorithm is only invoked in one place (the "create pooling operations" steps) can we just move it inline?

[huningxin]

Should we add validation steps for user-supplied outputSizes? Chromium prototype does that: https://source.chromium.org/chromium/chromium/src/+/main:services/webnn/public/cpp/graph_validation_utils.cc;l=1179;drc=60130be50f68c89220d8ace62c87155025267a2c;bpv=1;bpt=1

If we add outputSizes validation steps, windowDimensions, padding, strides and dilations parameters would be required.

[jlewis-austin]

The outputSizes validation above in 13.2 doesn't appear to be correct--shouldn't it be similar to this logic?