Skip to content

Commit

Permalink
Bug fix: Fix MLGraphBuilder.input()'s handling of scalars (#575)
Browse files Browse the repository at this point in the history
* Bug fix: Fix MLGraphBuilder.input()'s handling of scalars. Fixes #502

MLOperandDescriptor was updated in c320472 to always have dimensions,
defaulted to an empty list for scalars. That makes the current prose
for input() incorrect. Issue #502 already tracked correcting it, so
let's simplify - just change the logic for "is a scalar?" and drop the
bogus assert.

* Remove conditional and fix check dimensions to allow 0
* Factor byte length check into checking dimensions
* Oops, this should go too.
* type -> dataType
* (1) simplify (2) move into MLOperandDescriptor (3) link to #456
* Restore dimension check and reorder byte length to last
* Fix build for missing dimensions

---------

Co-authored-by: Dwayne Robinson <[email protected]>
  • Loading branch information
inexorabletash and fdwr authored Feb 21, 2024
1 parent 4ad9a63 commit 6e99b01
Showing 1 changed file with 21 additions and 19 deletions.
40 changes: 21 additions & 19 deletions index.bs
Original file line number Diff line number Diff line change
Expand Up @@ -1604,8 +1604,7 @@ Create a constant {{MLOperand}} of the specified data type and shape that contai
<div class="note">
The permissions and context validity have been checked by [[#api-mlgraphbuilder-constructor]] steps.
</div>
1. If |descriptor|'s [=byte length=] is not supported by the underlying platform, then [=exception/throw=] a "{{DataError}}" {{DOMException}}.
1. If the [=checking dimensions=] given |descriptor|.{{MLOperandDescriptor/dataType}} and |descriptor|.{{MLOperandDescriptor/dimensions}} returns false, then [=exception/throw=] a "{{DataError}}" {{DOMException}}.
1. If [=checking dimensions=] given |descriptor| returns false, then [=exception/throw=] a "{{DataError}}" {{DOMException}}.
1. If [=validating buffer with descriptor=] given |bufferView| and |descriptor| returns false, then [=exception/throw=] a {{TypeError}}.
1. If any of the following sub-steps fail, [=exception/throw=] an "{{OperationError}}" {{DOMException}}.
1. Let |operand| be the result of [=creating an MLOperand=] given [=this=] and |descriptor|.
Expand Down Expand Up @@ -3318,10 +3317,7 @@ Create a named {{MLOperand}} based on a descriptor, that can be used as an input
The permissions and context validity have been checked by [[#api-mlgraphbuilder-constructor]] steps.
</div>
1. If |name| is empty, then [=exception/throw=] a {{TypeError}}.
1. [=Assert=]: If |descriptor|.{{MLOperandDescriptor/dimensions}} does not [=map/exist=], then |descriptor| defines a scalar input.
1. If |descriptor|.{{MLOperandDescriptor/dimensions}} [=map/exists=]:
1. If [=checking dimensions=] given |descriptor|.{{MLOperandDescriptor/dataType}} and |descriptor|.{{MLOperandDescriptor/dimensions}} returns false, then [=exception/throw=] a "{{DataError}}" {{DOMException}}.
1. If |descriptor|'s [=byte length=] is not supported by the underlying platform, then [=exception/throw=] a "{{DataError}}" {{DOMException}}.
1. If [=checking dimensions=] given |descriptor| returns false, then [=exception/throw=] a "{{DataError}}" {{DOMException}}.
1. If any of the following sub-steps fail, [=exception/throw=] an "{{OperationError}}" {{DOMException}}.
1. Let |operand| be the result of [=creating an MLOperand=] given [=this=] and |descriptor|.
1. Set |operand|.{{MLOperand/[[name]]}} to |name|.
Expand Down Expand Up @@ -5781,26 +5777,14 @@ The {{MLOperand}} objects are created by the methods of {{MLGraphBuilder}}, inte
</div>
</details>

<details open algorithm>
<summary>
To <dfn>check dimensions</dfn> given [=/list=] |dimensions| and {{MLOperandDataType}} |type|, run the following steps:
</summary>
<div class=algorithm-steps>
1. If |dimensions|'s [=list/size=] is 0, return false.
1. If |dimensions|'s [=list/size=] is too large to be supported by the implementation, return false.
1. If any element of |dimensions| is not a positive number, or it is too large to be supported by the implementation given |type|, return false.
1. Return true.
</div>
</details>

<details open algorithm>
<summary>
To <dfn for="MLOperand">validate MLOperand</dfn> given {{MLOperand}} |operand| and {{MLGraphBuilder}} |builder|, run the following steps:
</summary>
<div class=algorithm-steps>
1. If |builder| is not equal to |operand|.{{MLOperand/[[builder]]}}, return false.
1. Let |desc| be |operand|.{{MLOperand/[[descriptor]]}}.
1. If [=checking dimensions=] given |desc|.{{MLOperandDescriptor/dimensions}} and |desc|.{{MLOperandDescriptor/dataType}} returns false, then return false.
1. If [=checking dimensions=] given |desc| returns false, then return false.
1. Return true.
</div>
</details>
Expand Down Expand Up @@ -5889,6 +5873,24 @@ dictionary MLOperandDescriptor {
</div>
</details>

<details open algorithm>
<summary>
To <dfn for="MLOperandDescriptor">check dimensions</dfn> given {{MLOperandDescriptor}} |descriptor|, run the following steps:
</summary>
<div class=algorithm-steps>
1. If any element of |descriptor|.{{MLOperandDescriptor/dimensions}} is too large to be supported by the implementation, return false.
1. If |descriptor|.{{MLOperandDescriptor/dimensions}}'s [=list/size=] is too large to be supported by the implementation, return false.

Issue(456): The maximum number of operand dimensions is not defined, but native ML APIs usually have a maximum supported size.

1. If |descriptor|'s [=byte length=] is not supported by the implementation, then return false.
1. Return true.
</div>


</details>


Algorithms {#algorithms}
=====================

Expand Down

0 comments on commit 6e99b01

Please sign in to comment.