webmachinelearning / webnn

🧠 Web Neural Network API
https://www.w3.org/TR/webnn/
Other
397 stars 48 forks source link

Is "validate graph resources" backwards? #602

Closed inexorabletash closed 7 months ago

inexorabletash commented 8 months ago

Currently, compute() calls validate graph resources with the passed input (or output) maps and the graph's input (or output) descriptors.

I believe the intent is to ensure that all of a graph's input (or output) descriptors are matched to a (name, buffer) pair in the passed input (or output) resources.

The steps are:

To validate graph resources, given MLNamedArrayBufferViews resources and ordered map descriptors, run the following steps:

  1. For each name → resource of resources:
    1. If descriptors[name] does not exist, return false.
    2. If validating buffer with descriptor given resource and descriptors[name] returns false, then return false.
  2. Return true.

This instead seems to be ensuring that each (name, buffer) pair in the passed resources has a corresponding input (or output) descriptor - which is similar but backwards.

i.e.

const builder = new MLGraphBuilder(context);
const input1 = builder.input('input1', desc1);
const input2 = builder.input('input2', desc2);
const output1 = builder.mul(input1, input2);
const graph = await builder.build({'output1': output1});
// graph has two inputs (input1, input2) and one output (output1)

// Expected usage
await context.compute(graph, {input1: inbuf1, input2: inbuf2}, {output1: outbuf1});

// Should fail, because input2 is missing - but it *doesn't*.
await context.compute(graph, {input1: inbuf1}, {output1: outbuf1});

// Maybe fail, because output2 is passed but unused. Currently it *does* fail. 
await context.compute(graph, {input1: inbuf1, input2: inbuf2}, {output1: outbuf1, output2: outbuf2});

It seems like we've got two options:

inexorabletash commented 8 months ago

It looks like Chromium does a reflexive test - I'll put up a PR for that.

inexorabletash commented 8 months ago

Note that in the PR #622 we ended up with a different approach, not a reflexive test. So please take a look over there.