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.

Sign up for GitHub

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

Jump to bottom

[Base] make ComputeImpl return MaybeError #233

Merged
merged 1 commit into from
Apr 20, 2022
Merged

[Base] make ComputeImpl return MaybeError #233

merged 1 commit into from
Apr 20, 2022

Conversation

lisa0314
Copy link
Collaborator

No description provided.

@lisa0314 lisa0314 force-pushed the main branch 2 times, most recently from 6e1b690 to 75617db Compare April 12, 2022 07:05
@lisa0314 lisa0314 requested a review from fujunwei April 12, 2022 08:11
@lisa0314
Copy link
Collaborator Author

@fujunwei PTAL. Thanks!

fujunwei
fujunwei approved these changes Apr 13, 2022
View reviewed changes
Copy link
Collaborator

@fujunwei fujunwei left a comment

Choose a reason for hiding this comment

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

LGTM with some comments.

Please verify if it can work in chromium, You can refer the comments from https://github.com/otcshare/webnn-native/issues/925#issuecomment-1097453086, thanks.

src/webnn_native/Graph.cpp Outdated
}

return ComputeImpl(inputs, outputs);
if (GetContext()->ConsumedError(ComputeImpl(inputs, outputs))) {
dawn::ErrorLog() << "fail to compute graph";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we can remove the log messages and return directly, ConsumedError will output error message in SetUncapturedErrorCallback

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I agree.

src/webnn_native/Graph.cpp Outdated
if (inputs == nullptr || outputs == nullptr) {
return WNNComputeGraphStatus_Error;
GetContext()->ConsumedError(DAWN_VALIDATION_ERROR("input or output is nullptr."));
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we can remove L.142 because it will create error object if there is an error for the object, so the pointer of NamedInputsBase and NamedOutputsBase aren't nullptr

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok

src/webnn_native/dml/GraphDML.cpp Outdated
auto namedInputs = inputs->GetRecords();
for (auto& [name, inputBinding] : mInputBindingMap) {
// All the inputs must be set.
if (namedInputs.find(name) == namedInputs.end()) {
dawn::ErrorLog() << "The input must be set.";
return WNNComputeGraphStatus_Error;
return DAWN_INTERNAL_ERROR("All inputs must be set.");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use DAWN_INVALID_IF macro instead of L.1821 and 1822

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good suggestion!

src/webnn_native/mlas/GraphMLAS.cpp Outdated
for (auto& [name, input] : inputs->GetRecords()) {
Ref<Memory> inputMemory = mInputs.at(name);
auto& resource = input.resource.arrayBufferView;
if (inputMemory->GetByteLength() < resource.byteLength) {
dawn::ErrorLog() << "The size of input memory is less than input buffer.";
return WNNComputeGraphStatus_Error;
return DAWN_INTERNAL_ERROR("The size of input memory is less than input buffer.");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use DAWN_INVALID_IF

src/webnn_native/mlas/GraphMLAS.cpp Outdated
@@ -1005,13 +1004,12 @@ namespace webnn_native::mlas {
Ref<Memory> outputMemory = mOutputs.at(outputName);
const ArrayBufferView& output = outputs->GetRecords().at(outputName).arrayBufferView;
if (output.byteLength < outputMemory->GetByteLength()) {
dawn::ErrorLog() << "The size of output buffer is less than output memory.";
return WNNComputeGraphStatus_Error;
return DAWN_INTERNAL_ERROR("The size of output buffer is less than output memory.");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use DAWN_INVALID_IF

@lisa0314 lisa0314 force-pushed the main branch 5 times, most recently from af19a53 to 9f73546 Compare April 19, 2022 06:58
@lisa0314
Copy link
Collaborator Author

@fujunwei this patch has been validated on chromium build. Please review again and help to merge. Thanks!

@fujunwei
Copy link
Collaborator

Merge it.

@fujunwei fujunwei merged commit c60a24b into webmachinelearning:main Apr 20, 2022
bbernhar pushed a commit to bbernhar/webnn-native that referenced this pull request Jun 28, 2022
@fujunwei
Add node and build script implementation
38a7261 
[Test] Add validation for Model

[Node] Refactor binding.gyp

Merge pull request webmachinelearning#233 from otcshare/mingming/lenet

[LeNet] Add the “-n” option to run the inference n times and print out the mean time

[Node] Ops test can workable on Linux

Merge pull request webmachinelearning#250 from otcshare/junwei/node

[Node] Refactor binding.gyp

[LeNet] fix -n option accepts negative values

fix webmachinelearning#253

Merge pull request webmachinelearning#254 from otcshare/nhu/lenet_perf

[LeNet] fix -n option accepts negative values

[Node] Most ops test can run on Linux and Windows.

[node] Use naming style as Dawn for file name

Merge pull request webmachinelearning#272 from otcshare/lisha/node

[node] Use naming style as Dawn for file name

[base] Add difination of leakyRelu

Merge pull request webmachinelearning#276 from otcshare/lisha/leakyRelu

[base] Add definition of leakyRelu

Merge pull request webmachinelearning#257 from otcshare/junwei/node

[Node] Most ops test can run on Linux and Windows.

[Base] Add Concat operations

[IE] Implement Concat with nGraph

[IE]Implement LeakyRelu op

[test] Added LeakyReluTests.

[build] Prepared .gclient file for gclient sync.

[build] Optimized build function of scripts.

[build] Enabled package function of build scripts.

[build] Enabled upload function of build scripts.

[build] Updated README.md for build scripts.

Merge pull request webmachinelearning#186 from otcshare/df/package-upload-build

[build] Supported to package and upload for build.

[DML] Support Concat operation

[Node] Reuse tests of webnn-polyfill

[build] Enabled notify function of build scripts.

add the asyncworker for compilation

[build] Fixed an issue of rewriting config file.

Merge pull request webmachinelearning#277 from otcshare/df/port_leakyRelu_tests

[test] Added LeakyReluTests.

[DML] Use max dimenssions count

delete friend class CompilationAsyncworker and use std::move to pass value

[build] Removed ienn lib build process.

Merge pull request webmachinelearning#280 from otcshare/junwei/ops

[Base] Add Concat operations

replace references of output_names_ with std::move in ComputeAsyncworker

Merge pull request webmachinelearning#288 from otcshare/kunliang/compilationAsync

add the asyncworker for compilation

[IE] Support depthwise conv2d

Merge pull request webmachinelearning#287 from otcshare/df/build-notify

[build] Enabled notify function of build scripts.

Merge pull request webmachinelearning#278 from otcshare/junwei/dev

[IE] Support depthwise conv2d

[DML] Support LeakyRelu operation

Merge pull request webmachinelearning#302 from otcshare/junwei/ops

[DML] Support LeakyRelu operation

[examples] Print median execution time for LeNet example.
           fixes webmachinelearning#292

[Base] Move common verification to Operand Base

[ie]Add Gemm op

[test]Add GemmTests

[DML] Support Gemm operations

Merge pull request webmachinelearning#304 from otcshare/junwei/ops

[Base] Move common verification to Operand Base

Merge pull request webmachinelearning#299 from otcshare/df/median_value

[examples] Print median value for LeNet example.

[DML] List the operand like a, b, c

Merge pull request webmachinelearning#285 from otcshare/mingming/node

[Node] Reuse tests of webnn-polyfill

Merge pull request webmachinelearning#293 from otcshare/lisha/gemm

[ie]Add Gemm op

[IE] Update ienn binary

[Base] Add definition of Clamp

[IE] Implement Clamp

[End2End] Add end2end tests for Clamp

[Refactor] Refactor some codes

[refactor] Refactor to support undefined options

[DML]Add Clamp

[Refactor] Git cl format and remove some duplicate codes

Merge pull request webmachinelearning#284 from otcshare/mingming/ops

[IE/DML] Implement clamp op

[IE] Define BatchNorm and implement for IE

[IE] Refactor codes for BatchNorm

[test] Added BatchNormWithoutOptions test for batchNormalization op.

[refactor] Refactor to support undefined options

[DML] Implement batchNorm

[DML] Pass BatchNorm test cases

Merge pull request webmachinelearning#301 from otcshare/mingming/batchNorm

[IE/DML] Define BatchNorm and implement for IE

[IE] Update binary on Windows

[Node]Rename to align with Dawn name style

[Node]change code format to align with Dawn

[Node]Fix build fatal

[build_script] Enabled build scripts on execute mode for main branch.

[oneDNN] the initial commit

[oneDNN] implement the constant

[oneDNN] rename files with DNNL postfix

[oneDNN] implement Relu and Softmax

[oneDNN] implement Reshape

[oneDNN] introduce memory reinterprets

[oneDNN] implement transpose

[oneDNN] implement binary add and mul

[oneDNN] implement matmul

[oneDNN] implement conv2d

[oneDNN] implement pool2d

[oneDNN] rename Reorder to ReorderIfNeeded

[oneDNN] use CHECK and ReorderIfNeeded

[oneDNN] fix reshape by reordering to plain layout

[oneDNN] optimize constants reordering

[oneDNN] add download_onednn.py into DEPS

[oneDNN] fix download_onednn.py for Linux

[oneDNN] set the LD_LIBRARY_PATH for Linux

[Base] returning error object for unimplemented ops

To avoid crash for new backend

[Test] assert compilation before use it

To avoid crash for failed test cases

[Node] webnn polyfill can run continuously

[onednn]Fix a typo

[IE]Add support nhwc layout for conv, pool, bn and add filterlayout
definition

[OneDNN]Fix build error caused by spec change

Updated ie_nn_c_api.dll file.

[tools] Implemented nightly auto test framework.

[tools] Move nightly-test scripts into build_script/tools folder.

add op Concat and fix compile error

update code based on comments

fix the format of code

[XNNPACK] The initial commit

[XNNPACK] implement unary ops

[XNNPACK] implement add and mul

[XNNPACK] implement the 2d matmul

[XNNPACK] reimplement relu by xnn_operator_t

Support the preallocated output buffer

[XNNPACK] implement clamp

[XNNPACK] implement add and mul

[XNNPACK] implement sub and tests

[XNNPACK] implement conv2d nhwc

[XNNPACK] supports filter layout ohwi of conv2d

[XNNPACK] implement fused conv2d

[XNNPACK] implement the depthwise conv2d

[Test] Add the SubTests.cpp

[XNNPACK] build on Linux

[XNNPACK] update the README.md

[Test] git cl format SubTests.cpp

[XNNPACK] implement pool2d nhwc

[Node]Clean license with Apache 2.0

[test] Updated layout value of conv2d, pool2d options.

[XNNPACK] fix the input layout and filter layout

[Test] add hwio and oihw filter layout for nhwc tests

[Test] fix the review comments

add Op BatchNorm

fix the format

modify the code based on comments

add Op LeakyRelu

modify the code based on comments

use the options with default value for Op LeakyRelu

modify the format for LeakyRelu

add op Gemm

fix the base on the comments

delet useless code

rebase commits and modify the code based on comments

keep the options only

[SqueezeNet] Add SqueezeNet(NCHW) C++ example

[SueezeNet] Support both NCHW/NHWC layout and refactor example codes

[Examples] Refactor codes

[Examples] Pass SqueezeNet NHWC and enable input parameters

[Examples] Add readme for SqueezeNet and refactor codes

[DML] Support NHWC layout

refactor code to simplify

[fix issue] Fix issue of compiling on Windows

[Fix issue] Fix stb loading issue

add op kunliang/clamp

modify the implementation for op Clamp based on comments

support Scalar argurement for Constants on Node.js

add more type for Constant

add three private variable for different dataTpye in Constant

add Union Scalar and remove the reinterpret_cast for each type

modify Constants based on comments

[example] Keep one images folder under examples

[modelTest] Add model tests for SqueezeNet NCHW/NHWC

[IENN]Fix the issue caused by nhwc support

Updated ie_nn_c_api.dll for Windows platform.

[tool] Update nightly-test tool to test SqueezeNet examples.

[build] Add linter for checking js files.

[build] Add SqueezeNet example into build package.

[build] Enable build for oneDNN backend on Windows platform.

[build] Enable build for XNNPACK backend.

Add NHWC support for batchNormalize

[autoPad] add autoPad for conv2d/pool2d and support conv2d autoPad for IE

[autoPad] Add end2end tests for conv2d autoPad

[build] Add test data files of SqueezeNet model tests into build package.

[tool] Updated nightly test script to test examples
       with changed paths of model and weights.

[Node] Refer the constant avoid garbage collection

[git config] Add third_party/stb into .gitignore

[Node] Add README and the steps of building node addon

[IE]Fix crash issue for unimplement binary ops

[node] fix the coding style issue

fix #451

[node] fix warnings

[DML]Add support for autopad of Conv2d

Update ie dll with batchnorm nhwc implement

[mobilenetv2] Add mobilenetv2 for nchw at first

[mobilenetv2] Refactor codes and support nchw/nhwc

[Refactor] Refactor examples codes

[DML]Fix the issue for running squeezentnhwc

[Rebase] Implement new Spec API
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fujunwei fujunwei fujunwei approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@lisa0314 @fujunwei