Add default values of scales into resample op #204
Conversation
| @@ -1702,7 +1702,7 @@ partial interface MLGraphBuilder { | |||
| - *options*: an optional {{MLResampleOptions}}. The optional parameters of the operation. | |||
| - *mode*: an {{MLInterpolationMode}}. The interpolation algorithm used to fill the output tensor values. | |||
| If not set, it is assumed to be the *Nearest Neighbor* interpolation. | |||
| - *scales*: a sequence of {{float}} of length 4. Each value represents the scaling factor used to scale in each input dimensions. | |||
| - *scales*: a sequence of {{float}} of length 4. Each value represents the scaling factor used to scale in each input dimensions. If not set, the values are assumed to be [1.0, 1.0, 1.0, 1.0]. | |||
There was a problem hiding this comment.
I'll loop in @dontcallmedom for his advice, given this is an instantiation of #205 (comment)
We have a sequence in an IDL fragment, so we learned we cannot use the WebIDL default value shorthand ("=").
The custom prose the spec has been using is:
If X is not set, the values are assumed to be Y.
Should we use this or maybe something like the following:
The default value is Y.
Or perhaps a longer version that explicitly specifies the default value is used if the value is "not present" or "is invalid"? (I think we may want to branch into two in some cases.)
If X is not present or its value invalid, set X to [1.0, 1.0, 1.0, 1.0].
We expect to use the same pattern in many places so would like to identify the most concise way.
Some notes:
-
WebIDL spec had a present dfn for testing dictionary members, but it was removed at some point?
-
For attributes there invalid value default and missing value default microsyntaxes, but nothing for dictionaries?
There was a problem hiding this comment.
The custom prose the spec has been using is:
If X is not set, the values are assumed to be Y.
Should we use this or maybe something like the following:
The default value is Y.
This LGTM - paging @domenic and @TimothyXu to confirm the wording indeed works, and whether a non-empty array as a default value doesn't create other issues from an IDL perspective.
Or perhaps a longer version that explicitly specifies the default value is used if the value is "not present" or "is invalid"? (I think we may want to branch into two in some cases.)
The term "present" has been replaced by Infra's exists (in WebIDL and in specs that rely on this concept).
An "invalid" value would cause a TypeError so there is no need to describe that situation.
There was a problem hiding this comment.
This doesn't really work, no. You need to have a normative algorithm which checks if input["scales"] exists, and if not, use the defaults. I.e. something like
- Let scales be the list « 1.0, 1.0, 1.0, 1.0 ».
- If input["scales"] exists, then set scales to input["scales"].
(and maybe also validate that input["scales"] is of length 4, or whatever you need?)
This seems difficult since right now your spec has no algorithms (i.e. no method steps) for its operations, just a non-normative description of the inputs and return value.
There was a problem hiding this comment.
@huningxin, I was just thinking of suggesting the same, given we track #211 separately.
Please go ahead and merge.
SHA: 4fc42a4 Reason: push, by @huningxin Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
fix #192
@wchao1115 @pyu10055 @anssiko , PTAL. Thanks.
Preview | Diff