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

Add WebNN baseline implementation for first-wave ops #1

Merged
merged 53 commits into from
May 13, 2022

Conversation

huningxin
Copy link
Contributor

This PR implements 41 WebNN ops for first-wave models in JavaScript double-precision Number and calculations without depending on 3rd party libs. It could be used to generate the baseline results for WebNN conformance tests.

Fix webmachinelearning/webnn#245

@anssiko @dontcallmedom @wchao1115 @pyu10055 , PTAL.

/cc @BruceDai

huningxin and others added 30 commits January 29, 2022 07:54
For activations, support clamp, leakyRelu, relu and sigmoid
Bumps [actions/checkout](https://github.com/actions/checkout) from 2.3.4 to 2.4.0.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@v2.3.4...v2.4.0)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [actions/setup-node](https://github.com/actions/setup-node) from 2.4.0 to 2.5.1.
- [Release notes](https://github.com/actions/setup-node/releases)
- [Commits](actions/setup-node@v2.4.0...v2.5.1)

---
updated-dependencies:
- dependency-name: actions/setup-node
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
1. Throw new Error
2. Skip padding values
3. Add doc of computePaddingForAutoPad
Copy link
Member

@anssiko anssiko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @huningxin and @BruceDai for this significant work. This proposal passes my review from the procedural perspective:

@dontcallmedom
Copy link
Contributor

Great work! In order to help with the goal of this codebase (making it easy to review it), I've created separately a couple of additional commits that I think help in that direction.

They take the following steps:

  • move any code that isn't directly a WebNN operation (but rather support code for operations) into the src/lib directory
  • tersify the definitions of operations by
    • moving as much of the parameters normalization into the function signature using JavaScript deconstruction patterns (and other syntactical shortcuts)
    • moving all the validation of the parameters into a separate module, to have the main module focus only on the actual calculation

An alternative to the latter subbullet might be to get rid of parameters validation altogether - since the code isn't actually meant to be used in any production environment, it's not clear that the code adds a lot of value. The refactoring would make that a fairly easy additional change.

@huningxin
Copy link
Contributor Author

Thanks much @dontcallmedom !

  • move any code that isn't directly a WebNN operation (but rather support code for operations) into the src/lib directory

+1. In the implementation, some big ops are implemented by composing small ops. These small ops are also WebNN ops, like reshape, so please keep them in the src folder.

  • moving as much of the parameters normalization into the function signature using JavaScript deconstruction patterns (and other syntactical shortcuts)

+1. I like that.

  • moving all the validation of the parameters into a separate module, to have the main module focus only on the actual calculation

+1

An alternative to the latter subbullet might be to get rid of parameters validation altogether - since the code isn't actually meant to be used in any production environment, it's not clear that the code adds a lot of value.

It's a fair point.

However, I suppose the validation code would help test developer to verify the test cases. @BruceDai

Another perspective is it probably could help the spec editors to write the validation steps of a method, as webmachinelearning/webnn#210. @anssiko @wchao1115

@dontcallmedom
Copy link
Contributor

An alternative to the latter subbullet might be to get rid of parameters validation altogether - since the code isn't actually meant to be used in any production environment, it's not clear that the code adds a lot of value.

However, I suppose the validation code would help test developer to verify the test cases. @BruceDai

I can certainly relate to that - they did help mind find and fix bugs I introduced in the process of refactoring the code :)

Another perspective is it probably could help the spec editors to write the validation steps of a method, as webmachinelearning/webnn#210. @anssiko @wchao1115

indeed

@anssiko
Copy link
Member

anssiko commented Feb 7, 2022

If the editors feel the validation code helps them translate checks into corresponding spec validation steps, it sounds like a good reason to keep that part to facilitate this work. A separate validation module is a good idea too from a code organization perspective.

@huningxin
Copy link
Contributor Author

huningxin#1 merged. Thanks @dontcallmedom 's contribution that improves the reviewability much.

* @param {MLConv2dOptions} options
* @return {Tensor}
*/
export function conv2d(input, filter, {padding = [0, 0, 0, 0],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wchao1115, this function implements the steps of conv2d, just for your convivence, thanks!

@anssiko
Copy link
Member

anssiko commented May 12, 2022

@huningxin as per our discussion, please feel free to merge this PR now to unblock webmachinelearning/webnn#265

Thanks to all reviewers and special thanks to @huningxin and @BruceDai for preparing this initial contribution that is an important deliverable to demonstrate implementation experience and satisfy interoperability requirements.

@huningxin huningxin merged commit be21680 into webmachinelearning:main May 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The baseline implementation of WebNN ops
4 participants