Content: Define operand concept, simplify graph connection steps#591
Content: Define operand concept, simplify graph connection steps#591fdwr merged 12 commits intowebmachinelearning:mainfrom inexorabletash:content-programming-model-operators
Conversation
As discussed in #572: - Define an "operator" concept, with inputs, outputs, activations. - Defer "platform operator" and "platform operand" to build. - Standardize the graph connection steps across builder methods. - Simplify activation, input and constant steps. Not covered in this change: - Erroring if input's [[builder]] doesn't match `this` - Build algorithm - covered by #448 and #457 - gru() is missing steps to populate output. Added "Issue" note. - Introducing "Validate arguments" section for each method. - Introducing "Calculate output shape" section for each method. For #549 and #572.
|
One thing that wasn't discussed in #572 in detail is: what to do with I think the right answer is: validate in the caller (e.g. the actual method), just like |
|
Another slight tweak we might want to make to this - there are multiple styles used for declaring the operator. Here are all the distinct examples:
I'm not sure it really matters, but the "gru" style stands out as the odd duck. That said, there's something to be said for quoting the name, e.g. this reads strangely: "Let operator be an operator for the where operation..." Spelling is also all over the place, e.g. "Leaky RELU" vs. "LSTM cell" vs. "Gather" vs. "instance normalization" vs. "batchNormalization". |
That all is probably due to my sloppiness while mass-defining those algorithms at different times and fatigue levels :). |
In tip-of-tree, the "desc" MLOperandDescriptor is populated, used, then modified and conditionally used again (if `returnSequence` is true) when creating "output2". This was broken in previous commits in this branch. Restore the intent, using a separate "desc2" MLOperandDescriptor only conditionally populated and then conditionally used.
|
Thanks @inexorabletash ! I am not feeling well today. I'll catch up ASAP. |
fdwr
left a comment
fdwr
left a comment
There was a problem hiding this comment.
Looks correct to me. Being more concise is nice (less "Make a request to the underlying platform to..."). Let specification be a specification that has as much verbiage as is necessary to be clear but no more 😉. TY JB.
Co-authored-by: Ningxin Hu <[email protected]>
…s' into content-programming-model-operators
* Add other operand inputs * PreLU -> PReLU * Define split() output shapes calculations
huningxin
left a comment
huningxin
left a comment
There was a problem hiding this comment.
LGTM, thanks so much for this great effort @inexorabletash !
| semantics, with no side effects. | ||
| Each operation invocation conceptually returns a distinct new value, without | ||
| changing the value of any other {{MLOperand}}. | ||
| In WebNN, a [=computational graph=] is composed of <dfn>operators</dfn> which act on data, and are the nodes of the graph. {{MLOperand}}s are a representation of data that flows within the computational graph, and are the edges of the graph. {MLOperand}}s include a [=computational graph=]'s <dfn for="computational graph">input</dfn> values for inference, <dfn for="computational graph">constants</dfn> (including trained weights) used for inference, intermediate values (often referred to as activations) computed during inference, as well as the output values of inference. An [=operator=]'s <dfn for=operator>input</dfn> is one or more {{MLOperand}}s. An [=operator=]'s <dfn for=operator>output</dfn> is one or more {{MLOperand}}s. [=Operators=] have operator-specific parameters that control their behavior, which can include zero or more <dfn for=operator lt="activation|activation function">activation functions</dfn>, which are {{MLActivation}}s. |
There was a problem hiding this comment.
The only remaining thing is whether we should define outputs. Because it is used by build() method, we probably can defer it to #457.
There was a problem hiding this comment.
Agreed - I didn't want to define an unused term just yet, but that was the intention.
|
Okay, sounds like this is ready for a merge - @fdwr maybe one last look? |
@inexorabletash : 🔎👀 ETA 16:10... (Oxford comma for the clarity win 👍) |
SHA: 1d1b531 Reason: push, by fdwr Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
|
Great work everyone! I believe we were just able to squeeze these changes into the CR Snapshot release train before it departs. I expect the programming model section to be read by people outside this WG, so improvements there were timely. |
5 participants
As discussed in #572:
Not covered in this change:
thisFor #549 and #572.
Preview | Diff