Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update the programming model section #549

Closed
anssiko opened this issue Feb 1, 2024 · 7 comments
Closed

Update the programming model section #549

anssiko opened this issue Feb 1, 2024 · 7 comments

Comments

@anssiko
Copy link
Member

anssiko commented Feb 1, 2024

Review and update the programming model section to ensure it still reads well and is up to date with the rest of the specification. This section can be used as a place to define concepts that apply globally.

@anssiko
Copy link
Member Author

anssiko commented Feb 1, 2024

PR #540 defines global concepts "platform operator" and "platform operand".

@inexorabletash
Copy link
Member

inexorabletash commented Feb 1, 2024

Some terms that would be good to define:

  • "tensor" and "shape"
  • "connect", "input" and "output" (referenced in operand creation steps)
  • "register" (e.g. "register ... as a graph output", "register it as an activation to ...", "register ... as a tensor constant", etc.)

@inexorabletash
Copy link
Member

If you're following this issue, please review: #572 (comment)

@anssiko
Copy link
Member Author

anssiko commented Feb 26, 2024

Programming Model changes in #572 (comment) LGTM.

Edit: Should we also say the nodes of the graph are connected by edges, represented by MLOperands? This is complementary to "MLOperands representing the data flow" but using graph theory language.

@inexorabletash
Copy link
Member

Edit: Should we also say the nodes of the graph are connected by edges, represented by MLOperands? This is complementary to "MLOperands representing the data flow" but using graph theory language.

Updated to read:

An operator's input is one or more MLOperands representing the data flow for the computation, and are the edges of the graph. Operands include input values...

inexorabletash added a commit to inexorabletash/webnn that referenced this issue Feb 28, 2024
As discussed in webmachinelearning#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 webmachinelearning#448 and webmachinelearning#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 webmachinelearning#549 and webmachinelearning#572.
fdwr pushed a commit that referenced this issue Mar 12, 2024
* Content: Define operand concept, simplify graph connection steps

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.

* Trivial fixes - missing spaces, wrong operation name

* lstm(): Fix output2 shape calculation

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.

* Update index.bs

Co-authored-by: Ningxin Hu <[email protected]>

* Initial review feedback:

* Add other operand inputs
* PreLU -> PReLU
* Define split() output shapes calculations

* Introduce definition for computational graph with inputs and constants

* Revise Programming Model section

---------

Co-authored-by: Ningxin Hu <[email protected]>
@inexorabletash
Copy link
Member

@anssiko - now that #591 is merged, do you think we can close this out and have more specific issues if needed?

@anssiko
Copy link
Member Author

anssiko commented Mar 13, 2024

Yes we can. Much thanks for doing a pass also on this part of the spec that I believe will be read by many web developers and people interested in the lay of the land.

@anssiko anssiko closed this as completed Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants