Restrict scale of dequantizeLinear and quantizeLinear to be positive #906
Conversation
index.bs
Outdated
| 1. If [=MLGraphBuilder/validating operand=] with [=this=] and any of |input|, |scale|, and |zeroPoint| returns false, then [=exception/throw=] a {{TypeError}}. | ||
| 1. If |input|'s [=MLOperand/dataType=] is not one of its [=/allowed data types=] (according to [this table](#tensor-limits-quantizelinear)), then [=exception/throw=] a {{TypeError}}. | ||
| 1. If |scale|'s [=MLOperand/dataType=] is not one of its [=/allowed data types=] (according to [this table](#tensor-limits-quantizelinear)), then [=exception/throw=] a {{TypeError}}. | ||
| 1. If |scale|'s values contain non-positive values, then [=exception/throw=] a {{TypeError}}. |
There was a problem hiding this comment.
given scale is an MLOperand, not sure if we can synchronously throw a TypeError, . Is it ok to say if the scale contains non-positive values, it's implementation dependent on how the error is handled?
There was a problem hiding this comment.
Yeah, the spec should just say they are unsupported in prose. We can't actually validate this until runtime.
There was a problem hiding this comment.
...unless we introduce the concept of an MLOperand parameter that must be a build-time constant.
There was a problem hiding this comment.
unless we introduce the concept of an MLOperand parameter that must be a build-time constant
There still remain dynamic cases when these inputs can be computed from another operator's output, but a constant property in the MLOperand may be informative for optimization purposes - for some precedent, there's already the isConstant slot and constant property getter from CreateConstantTensor.
There was a problem hiding this comment.
Thanks for your reviewing! I've updated these descriptions in new commit, please take another look! ☕
There was a problem hiding this comment.
it's implementation dependent on how the error is handled
@philloooo: 👀I see one existing case with argMin/argMax, "In case of ties, the identity of the return value is implementation dependent". So we could update the current text "Values must be positive and nonzero" to something more like "Values must be positive and nonzero, or else results are implementation dependent"?
4 participants
This is a Spec change PR for #879 where we're discussing about the lack of widespread support for the current negative scale of
dequantizeLinearandquantizeLinearacross all backend.@fdwr @reillyeon @huningxin PTAL, thanks!
Preview | Diff