From e035edeb4877c34fa7cbfe290fa85c67e2dac36a Mon Sep 17 00:00:00 2001 From: Matthew Soulanille Date: Thu, 4 May 2023 11:58:49 -0700 Subject: [PATCH] Remove duplicate Prod and SparseToDense ops from converter (#7649) Fixes #7648 --- .../tensorflowjs/op_list/basic_math.json | 30 ---------------- .../tensorflowjs/op_list/normalization.json | 35 ------------------- .../tensorflowjs/op_list/reduction.json | 6 ++++ .../executors/basic_math_executor.ts | 4 --- .../executors/basic_math_executor_test.ts | 20 +---------- .../executors/normalization_executor.ts | 12 +------ .../executors/normalization_executor_test.ts | 29 --------------- .../executors/reduction_executor_test.ts | 11 +++++- .../src/operations/operation_mapper_test.ts | 5 ++- 9 files changed, 22 insertions(+), 130 deletions(-) diff --git a/tfjs-converter/python/tensorflowjs/op_list/basic_math.json b/tfjs-converter/python/tensorflowjs/op_list/basic_math.json index f530752ed04..d63b255c66c 100644 --- a/tfjs-converter/python/tensorflowjs/op_list/basic_math.json +++ b/tfjs-converter/python/tensorflowjs/op_list/basic_math.json @@ -796,36 +796,6 @@ } ] }, - { - "tfOpName": "Prod", - "category": "basic_math", - "inputs": [ - { - "start": 0, - "name": "x", - "type": "tensor" - }, - { - "start": 1, - "name": "axes", - "type": "number[]" - } - ], - "attrs": [ - { - "tfName": "keep_dims", - "name": "keepDims", - "type": "bool", - "notSupported": true - }, - { - "tfName": "T", - "name": "dtype", - "type": "dtype", - "notSupported": true - } - ] - }, { "tfOpName": "LeakyRelu", "category": "basic_math", diff --git a/tfjs-converter/python/tensorflowjs/op_list/normalization.json b/tfjs-converter/python/tensorflowjs/op_list/normalization.json index 535b7167374..fd460f0da3a 100644 --- a/tfjs-converter/python/tensorflowjs/op_list/normalization.json +++ b/tfjs-converter/python/tensorflowjs/op_list/normalization.json @@ -216,40 +216,5 @@ "type": "tensor" } ] - }, - { - "tfOpName": "SparseToDense", - "category": "normalization", - "inputs": [ - { - "start": 0, - "name": "sparseIndices", - "type": "tensor" - }, - { - "start": 1, - "name": "outputShape", - "type": "number[]" - }, - { - "start": 2, - "name": "sparseValues", - "type": "tensor" - }, - { - "start": 3, - "name": "defaultValue", - "type": "tensor" - } - ], - "attrs": [ - { - "tfName": "validate_indices", - "name": "validateIndices", - "type": "bool", - "defaultValue": true, - "notSupported": true - } - ] } ] diff --git a/tfjs-converter/python/tensorflowjs/op_list/reduction.json b/tfjs-converter/python/tensorflowjs/op_list/reduction.json index bbb0cfdae1e..486a75461cb 100644 --- a/tfjs-converter/python/tensorflowjs/op_list/reduction.json +++ b/tfjs-converter/python/tensorflowjs/op_list/reduction.json @@ -238,6 +238,12 @@ "tfName": "keep_dims", "name": "keepDims", "type": "bool" + }, + { + "tfName": "T", + "name": "dtype", + "type": "dtype", + "notSupported": true } ] }, diff --git a/tfjs-converter/src/operations/executors/basic_math_executor.ts b/tfjs-converter/src/operations/executors/basic_math_executor.ts index 6f58abf7ee1..530cd775791 100644 --- a/tfjs-converter/src/operations/executors/basic_math_executor.ts +++ b/tfjs-converter/src/operations/executors/basic_math_executor.ts @@ -159,10 +159,6 @@ export const executeOp: InternalOpExecutor = getParamValue('x', node, tensorMap, context) as Tensor)]; case 'Rsqrt': return [ops.rsqrt(getTensor(node.inputNames[0], tensorMap, context))]; - case 'Prod': - return [ops.prod( - getParamValue('x', node, tensorMap, context) as Tensor, - getParamValue('axes', node, tensorMap, context) as number[])]; case 'LeakyRelu': return [ops.leakyRelu( getParamValue('x', node, tensorMap, context) as Tensor, diff --git a/tfjs-converter/src/operations/executors/basic_math_executor_test.ts b/tfjs-converter/src/operations/executors/basic_math_executor_test.ts index 4154d2a6825..5d545d4a24f 100644 --- a/tfjs-converter/src/operations/executors/basic_math_executor_test.ts +++ b/tfjs-converter/src/operations/executors/basic_math_executor_test.ts @@ -24,7 +24,7 @@ import {Node} from '../types'; import {executeOp} from './basic_math_executor'; import {RecursiveSpy, spyOnAllFunctions} from './spy_ops'; -import {createNumberAttr, createNumberAttrFromIndex, createNumericArrayAttrFromIndex, createTensorAttr, uncapitalize, validateParam} from './test_helper'; +import {createNumberAttr, createNumberAttrFromIndex, createTensorAttr, uncapitalize, validateParam} from './test_helper'; describe('basic math', () => { let node: Node; @@ -108,24 +108,6 @@ describe('basic math', () => { expect(validateParam(node, basic_math.json)).toBeTruthy(); }); }); - describe('Prod', () => { - it('should call tfOps.prod', () => { - node.op = 'Prod'; - node.inputParams['axes'] = createNumericArrayAttrFromIndex(1); - node.inputNames = ['input1', 'input2']; - const input2 = [tfOps.tensor1d([2])]; - spyOps.prod.and.returnValue({}); - executeOp(node, {input1, input2}, context, spyOpsAsTfOps); - - expect(spyOps.prod).toHaveBeenCalledWith(input1[0], [2]); - }); - it('should match op def', () => { - node.op = 'Prod'; - node.inputParams['axes'] = createNumericArrayAttrFromIndex(1); - - expect(validateParam(node, basic_math.json)).toBeTruthy(); - }); - }); describe('Rsqrt', () => { it('should call tfOps.rsqrt', () => { const input1 = [tfOps.scalar(1)]; diff --git a/tfjs-converter/src/operations/executors/normalization_executor.ts b/tfjs-converter/src/operations/executors/normalization_executor.ts index 58b5e24f247..56abb03ee8f 100644 --- a/tfjs-converter/src/operations/executors/normalization_executor.ts +++ b/tfjs-converter/src/operations/executors/normalization_executor.ts @@ -15,7 +15,7 @@ * ============================================================================= */ -import {Scalar, Tensor, Tensor3D, Tensor4D} from '@tensorflow/tfjs-core'; +import {Tensor, Tensor3D, Tensor4D} from '@tensorflow/tfjs-core'; // tslint:disable-next-line: no-imports-from-dist import * as tfOps from '@tensorflow/tfjs-core/dist/ops/ops_for_converter'; @@ -70,16 +70,6 @@ export const executeOp: InternalOpExecutor = return [ops.logSoftmax( getParamValue('x', node, tensorMap, context) as Tensor)]; } - case 'SparseToDense': { - return [ops.sparseToDense( - getParamValue('sparseIndices', node, tensorMap, context) as - Tensor, - getParamValue('outputShape', node, tensorMap, context) as Tensor, - getParamValue('sparseValues', node, tensorMap, context) as - number[], - getParamValue('defaultValue', node, tensorMap, context) as - Scalar)]; - } default: throw TypeError(`Node type ${node.op} is not implemented`); } diff --git a/tfjs-converter/src/operations/executors/normalization_executor_test.ts b/tfjs-converter/src/operations/executors/normalization_executor_test.ts index 484836a99ec..1e53b9346ac 100644 --- a/tfjs-converter/src/operations/executors/normalization_executor_test.ts +++ b/tfjs-converter/src/operations/executors/normalization_executor_test.ts @@ -185,35 +185,6 @@ describe('normalization', () => { it('should match json def', () => { node.op = 'LogSoftmax'; - expect(validateParam(node, normalization.json)).toBeTruthy(); - }); - }); - describe('SparseToDense', () => { - it('should call tfOps.sparseToDense', () => { - node.op = 'SparseToDense'; - node.inputParams.sparseIndices = createTensorAttr(0); - node.inputParams.outputShape = createNumericArrayAttrFromIndex(1); - node.inputParams.sparseValues = createTensorAttr(2); - node.inputParams.defaultValue = createTensorAttr(3); - node.inputNames = ['input1', 'input2', 'input3', 'input4']; - const input2 = [tfOps.tensor1d([1], 'int32')]; - const input3 = [tfOps.scalar(2)]; - const input4 = [tfOps.scalar(3)]; - spyOps.sparseToDense.and.returnValue({}); - executeOp(node, {input1, input2, input3, input4}, context, - spyOpsAsTfOps); - - expect(spyOps.sparseToDense) - .toHaveBeenCalledWith(input1[0], [1], input3[0], input4[0]); - }); - it('should match json def', () => { - node.op = 'SparseToDense'; - delete node.inputParams.x; - node.inputParams.sparseIndices = createTensorAttr(0); - node.inputParams.outputShape = createNumericArrayAttrFromIndex(1); - node.inputParams.sparseValues = createTensorAttr(2); - node.inputParams.defaultValue = createTensorAttr(3); - expect(validateParam(node, normalization.json)).toBeTruthy(); }); }); diff --git a/tfjs-converter/src/operations/executors/reduction_executor_test.ts b/tfjs-converter/src/operations/executors/reduction_executor_test.ts index 9950e164016..06d0c1491a1 100644 --- a/tfjs-converter/src/operations/executors/reduction_executor_test.ts +++ b/tfjs-converter/src/operations/executors/reduction_executor_test.ts @@ -23,7 +23,7 @@ import {Node} from '../types'; import {executeOp} from './reduction_executor'; import {RecursiveSpy, spyOnAllFunctions} from './spy_ops'; -import {createBoolAttr, createNumberAttr, createNumberAttrFromIndex, createTensorAttr, uncapitalize, validateParam} from './test_helper'; +import {createBoolAttr, createNumberAttr, createNumberAttrFromIndex, createNumericArrayAttrFromIndex, createTensorAttr, uncapitalize, validateParam} from './test_helper'; describe('reduction', () => { let node: Node; @@ -156,5 +156,14 @@ describe('reduction', () => { .toBeTruthy(); }); }); + describe('Prod', () => { + it('should match op def', () => { + node.op = 'Prod'; + node.inputParams['axis'] = createNumericArrayAttrFromIndex(1); + node.attrParams['keepDims'] = createBoolAttr(true); + + expect(validateParam(node, reduction.json)).toBeTruthy(); + }); + }); }); }); diff --git a/tfjs-converter/src/operations/operation_mapper_test.ts b/tfjs-converter/src/operations/operation_mapper_test.ts index e9a2cb7bed8..522d95382b3 100644 --- a/tfjs-converter/src/operations/operation_mapper_test.ts +++ b/tfjs-converter/src/operations/operation_mapper_test.ts @@ -274,7 +274,10 @@ describe('completeness check', () => { }; convertedGraph = mapper.transformGraph(graph); expect(Object.keys(convertedGraph.nodes)).toEqual([tfOp.tfOpName]); - expect(convertedGraph.nodes[tfOp.tfOpName].op).toEqual(tfOp.tfOpName); + const node = convertedGraph.nodes[tfOp.tfOpName]; + expect(node.op).toEqual(tfOp.tfOpName); + expect(node.category).withContext(`Op: ${node.op}, category`) + .toEqual(tfOp.category); }); }); });