Implement ModelBuilder and update the execution API #23
Conversation
|
Thanks @BruceDai for contributing the nhwc conv2d and depthwise conv2d test cases. It helped me to find an issue of filter layout for nhwc depthwise conv2d. I pushed new comments to fix it. Please take a look. |
| const nn = navigator.ml.getNeuralNetworkContext(); | ||
| const builder = nn.createModelBuilder(); | ||
| const desc = {type: 'float32', dimensions: [2, 2]}; | ||
| const a = builder.input('a', desc); |
There was a problem hiding this comment.
operands seem to contain builder, can it provide chained API?
i.e. a.matmul(b)
There was a problem hiding this comment.
Created webmachinelearning/webnn#106 to follow-up in spec.
| const expectedE = [3, 3, 3, 3]; | ||
|
|
||
| it('Compilation should have compute method', async () => { | ||
| const model = builder.createModel({c}); |
There was a problem hiding this comment.
this is bit confusing, the builder creates models, but the topology is created before the model is created, and how made the topology immutable for the model?
There was a problem hiding this comment.
The model should be immutable after createModel. As discussed with @pyu10055 , it needs to add details of the createModel method into the spec. Created webmachinelearning/webnn#107 to follow up.
There was a problem hiding this comment.
I added a new commit cb283bd. It includes a test case for immutable model and fixes a bug that the constant operand is not immutable. Please take another look.
Add test cases for immutable model.
huningxin
force-pushed
the
modelbuilder
branch
from
e0ba924 to
cb283bd
Compare
|
@BruceDai , could you please verify whether the depthwise conv2d issue was fixed? Thanks! |
|
@huningxin I verified with three test cases converted from Android NNAPI CTS tests, now these test cases were all PASS, thanks for fixing this issue. BTW, I'm planning to submit a PR of adding some test cases converted from Android NNAPI CTS tests and the converter scripts to this repo, and hope to get some feedback from you and also the community, thanks. |
|
@huningxin I believe this PR has received adequate review and we can merge this now. |
fix #22
Implement the WebNN API change introduced by webmachinelearning/webnn#94.
Preview:
Detailed changes include:
@anssiko @wchao1115 @pyu10055 @BruceDai , please take a look. Thanks!