Remove duplicate Prod and SparseToDense ops from converter (#7649)

Fixes #7648
tensorflow · May 4, 2023 · e035ede · e035ede
1 parent d66c17b
commit e035ede
Show file tree

Hide file tree

Showing 9 changed files with 22 additions and 130 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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);
       });
     });
   });