Use unsigned integer axis

[huningxin]

fix #345, #317

This PR makes the axis/axes definitions consistent by using the unsigned integer type and valid value range [0, N-1] where N is the tensor rank across the spec.

@wchao1115 @wacky6 @fdwr @anssiko @pyu10055 @RafaelCintron , PTAL. Thanks!

---

Preview | Diff

[anssiko]

🚀

[fdwr]

@huningxin : Should we split the MLBatchNormalizationOptions default into a separate PR for clarity from the signed -> unsigned aspect?

[fdwr]

Thanks. Minor proposals, but I'm also content as-is. Rescanning the spec, it appears you got all the "long axis" and "sequence" occurrences. 👍

[huningxin]

@fdwr

@huningxin : Should we split the MLBatchNormalizationOptions default into a separate PR for clarity from the signed -> unsigned aspect?

sgtm. I removed the change of MLBatchNormalizationOptions.axis default value in the latest commit. Please take another look. Thanks!

[fdwr]

LGTM. TY.

[huningxin]

Thanks for the review and approvals. With that, I am going to merge this PR.

[fdwr]

@huningxin : I should have noticed earlier while reviewing, but I just noticed slices parameter starts still has the negative number policy and accepts axis coordinates as long rather than unsigned long; and its sizes parameter has an unusual policy that -1 is special, whereas sizes in resample2D is sequence<unsigned long>. Do we want to rationalize these for consistency too, having higher level policy like this determined by the calling framework?

[huningxin]

Thanks @fdwr . This PR focused on the axis/axes, so it just changed MLSliceOptions.axes to sequence<unsigned long>. For starts, because it is actually the offset along each dimension, it was left as-is. There was similar reason for unchanged sizes.

However, I agree with you that using unsigned long for starts and sizes may help the API consistency and simplify the implementation. The slice op of native ML APIs, like DirectML and XNNPACK, seem to only support unsigned starts and sizes. Another consideration would be whether we should simplify the pad by removing MLSliceOptions.axes. It can just require the length of starts and sizes is same as input tensor's rank.

I propose to open a separate issue to track it, WDYT?

[fdwr]

@huningxin : 👍 Separate issue - thanks.

may help the API consistency and simplify the implementation ...

Yeah, we see that every other sizes parameter is unsigned long (see below), leaving only slice as sequence<long> sizes.

dictionary MLResample2dOptions {
...
  sequence<unsigned long> sizes;
...
};

dictionary MLPool2dOptions {
  ...
  sequence<unsigned long> outputSizes;
};

dictionary MLPool2dOptions {
...
  sequence<unsigned long> outputSizes;
};

Another consideration would be whether we should simplify the pad by removing MLSliceOptions.axes. It can just require the length of starts and sizes is same as input tensor's rank.

🤔⏳ Not sure, as I want to know what XNNPack does first too (DML_SLICE1_OPERATOR_DESC expects all dimensions to be listed explicitly, and unchanged dimensions just use the full range), but I generally lean the direction of being more explicit for WebNN.

[huningxin]

@fdwr

as I want to know what XNNPack does first too

xnn_define_static_slice also expects all dimensions to be listed.

but I generally lean the direction of being more explicit for WebNN.

+1