Add support for NonMaxSuppressionV2 and NonMaxSuppressionV3

[pyu10055]

This change is 

[dsmilkov]

Reviewed 10 of 10 files at r1.
Reviewable status: 0 of 1 approvals obtained (waiting on @pyu10055 and @dsmilkov)

---

src/operations/operation_mapper.ts, line 38 at r1 (raw file):

const CONTROL_FLOW_OPS = ['Switch', 'Merge', 'Enter', 'Exit', 'NextIteration'];
const DYNAMIC_SHAPE_OPS = ['NonMaxSuppressionV2', 'NonMaxSuppressionV3'];

the kernel 'Where' is dynamic op as well. Can you add a mapping from 'Where' to tf.whereAsync(condition)

---

src/operations/executors/image_executor.ts, line 62 at r1 (raw file):

      const scoreThreshold =
          getParamValue('scoreThreshold', node, tensorMap, context) as number;
      return [tfc.image.nonMaxSuppression(

you will have to call tfc.nonMaxSuppressionAsync which returns Promise<Tensor> in order to avoid blocking the UI thread.

[pyu10055]

thanks, updated to use async version.

Reviewable status: 0 of 1 approvals obtained (waiting on @pyu10055)

---

src/operations/operation_mapper.ts, line 38 at r1 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

the kernel 'Where' is dynamic op as well. Can you add a mapping from 'Where' to tf.whereAsync(condition)

I will create a separate PR for where after this is checked in.

[dsmilkov]

Reviewed 1 of 6 files at r2.
Reviewable status: 0 of 1 approvals obtained (waiting on @pyu10055)

---

src/operations/operation_mapper.ts, line 38 at r1 (raw file):

Previously, pyu10055 (Ping Yu) wrote…

I will create a separate PR for where after this is checked in.

Sounds good

---

src/operations/executors/image_executor.ts, line 29 at r2 (raw file):

export let executeOp: OpExecutor = async(
    node: Node, tensorMap: NamedTensorsMap,
    context: ExecutionContext): Promise<tfc.Tensor[]> => {

I think this is not the way since now even calls to tf.resizeBilinear (which is not dynamic) will return Promise, instead of tensor, which means regular frozenModel.predict() will return Promise, instead of Tensor.

Once you mark a function as async, it will always return a Promise. Move nonMaxSupression to a separate dynamic_executor.ts, just like you do with control_executor. Also it's important that the unit tests didn't catch this bug. Which means we should test that the actual result returned from executeOp is of the expected type. See comment below where the unit test is.

---

src/operations/executors/image_executor_test.ts, line 81 at r2 (raw file):

        const input4 = [tfc.tensor1d([1])];
        spyOn(tfc.image, 'nonMaxSuppressionAsync');
        executeOp(node, {input1, input2, input3, input4}, context);

Can you also verify in the unit test that the result of this executeOp is a Promise, and not a Tensor. Likewise, for non dynamic ops, we should verify that the result of executeOp is a Tensor, and not a Promise.

See previous comment for motivation.

[pyu10055]

Reviewable status: 0 of 1 approvals obtained (waiting on @pyu10055)

---

src/operations/executors/image_executor_test.ts, line 81 at r2 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

Can you also verify in the unit test that the result of this executeOp is a Promise, and not a Tensor. Likewise, for non dynamic ops, we should verify that the result of executeOp is a Tensor, and not a Promise.

See previous comment for motivation.

Added test for non_max_suppression now, will update all tests in a separate PR.

[dsmilkov]

Nice work!!

Reviewed 9 of 10 files at r3.
Reviewable status: 0 of 1 approvals obtained (waiting on @dsmilkov)