Skip to content

Update the NSNet2 example and add its link #131

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

Merged
merged 3 commits into from
Feb 6, 2021

Conversation

huningxin
Copy link
Contributor

Update the NSNet2 example code base on https://webmachinelearning.github.io/webnn-samples/nsnet2/ and add the link into the explainer.

@wchao1115 @anssiko , please take a look. Thanks!

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! Suggested some minor changes to the text.

I assume this code snippet is functionally equivalent to the JS module implementation https://github.com/webmachinelearning/webnn-samples/blob/master/nsnet2/nsnet2.js so I suggested linking to that from below the code snippet.

Alternatively, we could replace the current snippet with the nsnet2.js code to have symmetry between the explainer and the running code sample. This way, we'd avoid the need to maintain two code paths for the same. WDYT?

explainer.md Outdated
@@ -74,90 +74,128 @@ This [example](https://webmachinelearning.github.io/webnn/#examples) builds, com

## Key scenarios

There are many important [application use cases](https://webmachinelearning.github.io/webnn/#usecases-application) for high-performance neural network inference. One such use cases is deep-learning noise suppression (DNS) in web-based video conferencing. The following sample shows how the NSNet baseline implementation of deep learning-based noise suppression model may be implemented as WebNN calls. The updated version of this model is available [here](https://github.com/microsoft/DNS-Challenge/tree/master/NSNet2-baseline).
There are many important [application use cases](https://webmachinelearning.github.io/webnn/#usecases-application) for high-performance neural network inference. One such use cases is deep-learning noise suppression (DNS) in web-based video conferencing. The following sample shows how the [NSNet2](https://github.com/microsoft/DNS-Challenge/tree/master/NSNet2-baseline) baseline implementation of deep learning-based noise suppression model may be implemented as WebNN calls.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
There are many important [application use cases](https://webmachinelearning.github.io/webnn/#usecases-application) for high-performance neural network inference. One such use cases is deep-learning noise suppression (DNS) in web-based video conferencing. The following sample shows how the [NSNet2](https://github.com/microsoft/DNS-Challenge/tree/master/NSNet2-baseline) baseline implementation of deep learning-based noise suppression model may be implemented as WebNN calls.
There are many important [application use cases](https://webmachinelearning.github.io/webnn/#usecases-application) for high-performance neural network inference. One such use case is deep-learning noise suppression (DNS) in web-based video conferencing. The following sample shows how the [NSNet2](https://github.com/microsoft/DNS-Challenge/tree/master/NSNet2-baseline) baseline implementation of deep learning-based noise suppression model may be implemented using the WebNN API.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion. Added a commit to fix it.

explainer.md Outdated
```

Try the live version of the [WebNN NSNet2 example](https://webmachinelearning.github.io/webnn-samples/nsnet2/).
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Try the live version of the [WebNN NSNet2 example](https://webmachinelearning.github.io/webnn-samples/nsnet2/).
Try the live version of the [WebNN NSNet2 example](https://webmachinelearning.github.io/webnn-samples/nsnet2/). This live version builds upon [nsnet2.js](https://github.com/webmachinelearning/webnn-samples/blob/master/nsnet2/nsnet2.js) that implements the above code snippet as a JS module.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed in a new commit.

@huningxin
Copy link
Contributor Author

I assume this code snippet is functionally equivalent to the JS module implementation https://github.com/webmachinelearning/webnn-samples/blob/master/nsnet2/nsnet2.js so I suggested linking to that from below the code snippet.

Good catch. Yes, they are functionally equivalent.

The major difference is that the nsnet2.js creates constants by loading the pre-trained data and dimensions from .npy files (buildConstantByNpy). The .npy files are dumped from the original onnx model.

On the other hand, the explainer sample code uses a more straightforward way to load pre-trained data from plain bin files. I think it might be simpler to read and have less dependency. Actually, to verify the code of this PR, I created another version of nsnet2.js that has the exact same implementation.

Alternatively, we could replace the current snippet with the nsnet2.js code to have symmetry between the explainer and the running code sample. This way, we'd avoid the need to maintain two code paths for the same. WDYT?

+1 to maintain the same code path.

There are two options:

  1. loading constants from .npy files (code of nsnet2.js)
  2. loading constants from plain bin files (code of this PR).

If you like #2, we can merge this PR and I can create another PR to update webnn-samples/nsnet2.

@anssiko
Copy link
Member

anssiko commented Jan 25, 2021

I was actually thinking the JS module implementation https://github.com/webmachinelearning/webnn-samples/blob/master/nsnet2/nsnet2.js (with .npy files and buildConstantByNpy) could be the code in the explainer as well, even if it is a bit longer and has some deps. I feel it has a more modern style and its encapsulation yields in more readable code.

But I'm fine either way, and don't feel strongly about this. Also, someone else might feel the opposite about these style aspects :-)

@wchao1115 thoughts?

@huningxin we can merge this to bake in the minor text updates and review the explainer code snippet as a JS module in a separate PR.

@wchao1115
Copy link
Collaborator

@anssiko Instead of replicating the same code in two places, we could copy just a small important excerpt e.g. the graph builder code from the JS module to the explainer, and accompanying it with a link to the full source code. Would that work?

@anssiko
Copy link
Member

anssiko commented Feb 4, 2021

@wchao1115 yes! That sounds like the best of both worlds approach.

@huningxin huningxin force-pushed the nsnet2 branch 3 times, most recently from 2fb74cc to 0ef9700 Compare February 6, 2021 03:33
@huningxin
Copy link
Contributor Author

huningxin commented Feb 6, 2021

@anssiko @wchao1115 , thanks for your suggestions. I optimized the implementation of NSNet2 example code and now it has only 60 lines of code. I also created a PR to webnn-samples webmachinelearning/webnn-samples#26. So the explainer and webnn-samples would use the exact same code.

Please take another look. Thanks!

@huningxin
Copy link
Contributor Author

Thanks for the review and approval.

@huningxin huningxin merged commit 356de2d into webmachinelearning:master Feb 6, 2021
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.

3 participants