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

WebNN binary tests #110

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

WebNN binary tests #110

wants to merge 3 commits into from

Conversation

BruceDai
Copy link
Owner

No description provided.

@@ -0,0 +1,133 @@
'use strict';

Choose a reason for hiding this comment

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

Please document the tfjs source and its Apache2 license in that file (in addition to the readme)

@@ -0,0 +1,7 @@
There are those following functions in ./utils.js copyed from

Choose a reason for hiding this comment

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

Suggested change
There are those following functions in ./utils.js copyed from
There are those following functions in ./utils.js copied from

getBroadcastDims()
assertAndGetBroadcastShape()
locToIndex()
indexToLoc()

Choose a reason for hiding this comment

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

];

// get baseline
const expected = baseline.binary(

Choose a reason for hiding this comment

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

as I alluded in webmachinelearning/webnn-polyfill#144 (comment), I wonder if it would not be better to provided hardcoded expected values rather than have them be calculated each time by the device under test (e.g. to avoid the case where the CPU or the JS engine itself are faulty). I'm not sure how likely that would be, so maybe this is not a high-priority concern.

@BruceDai
Copy link
Owner Author

Thanks much @dontcallmedom. I will address your comments, and update this PR.

@@ -1,5 +1,21 @@
/**
* Copyright 2021 WebNN Tests contributors
Copy link
Owner Author

Choose a reason for hiding this comment

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

@dontcallmedom

From https://github.com/tensorflow/tfjs/blob/master/tfjs-core/src/ops/broadcast_util.ts#L3

Copyright 2017 Google LLC. All Rights Reserved.

, I'm not sure that it's allowable to use the code of tensorflow/tfjs project. Any comment, thanks.

Choose a reason for hiding this comment

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

Since the files we're copying ares under Apache 2 license, as long as we keep the licensing info intact, there should be no issue with copying the code. But the copyright needs to remain to Google LLC (here and in all the other copied files)

Copy link
Owner Author

@BruceDai BruceDai left a comment

Choose a reason for hiding this comment

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

@dontcallmedom I updated the commit, PTAL, thanks.

@@ -1,5 +1,21 @@
/**
* Copyright 2021 WebNN Tests contributors

Choose a reason for hiding this comment

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

Since the files we're copying ares under Apache 2 license, as long as we keep the licensing info intact, there should be no issue with copying the code. But the copyright needs to remain to Google LLC (here and in all the other copied files)

@@ -0,0 +1,8 @@
There are those following functions in ./utils.js copied from
https://github.com/tensorflow/tfjs project which is licensed under the [LICENSE](https://github.com/tensorflow/tfjs/blob/master/LICENSE).

Choose a reason for hiding this comment

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

Suggested change
https://github.com/tensorflow/tfjs project which is licensed under the [LICENSE](https://github.com/tensorflow/tfjs/blob/master/LICENSE).
https://github.com/tensorflow/tfjs project which is licensed under the [Apache 2.0 License](https://github.com/tensorflow/tfjs/blob/master/LICENSE).

@huningxin
Copy link

We've started a new baseline implementation from scratch without referring 3rd party libraries. https://github.com/huningxin/webnn-baseline

I propose to move it to group repo for wider review and development: webmachinelearning/webnn#245

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