From 7cad3e506bc3a20b2ea1e066ae00e199f6907b39 Mon Sep 17 00:00:00 2001 From: Matthew Soulanille Date: Wed, 8 Mar 2023 17:05:21 -0800 Subject: [PATCH 1/2] Never pass a NoOp value to an op The 'NoOp' op appears in the execution graph (although it shouldn't really), and its output value can sometimes be passed to downstream nodes. Temporarily fix this by filtering it out of the parameters in `getParamValue`. TODO(mattSoulanille): Refactor the graph so NoOp and other control dependencies are only used for sorting and don't appear in the execution graph. --- tfjs-converter/src/operations/executors/utils.ts | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/tfjs-converter/src/operations/executors/utils.ts b/tfjs-converter/src/operations/executors/utils.ts index 17b5b7286ed..ee9157586f8 100644 --- a/tfjs-converter/src/operations/executors/utils.ts +++ b/tfjs-converter/src/operations/executors/utils.ts @@ -38,9 +38,16 @@ export function getParamValue( node.inputNames[shiftedStart], tensorMap, context, resourceManager); } if (inputParam.type === 'tensors') { - const inputs = node.inputNames.slice(start, end); - - return inputs.map( + // TODO(mattSoulanille): This filters out NoOp nodes during execution, but + // these should really never be in the execution graph in the first place. + // They're necessary for ordering the graph, but should not be visible + // during execution. Perhaps have different sets of children, one for + // control dependencies and another for real dependencies. + const inputs = node.inputs.slice(start, end); + const inputNames = inputs.filter(node => node.op !== 'NoOp') + .map(node => node.name); + + return inputNames.map( name => getTensor(name, tensorMap, context, resourceManager)); } const tensor = getTensor( From ad2a618fda4682a8de7d857d81dde217ca698341 Mon Sep 17 00:00:00 2001 From: Matthew Soulanille Date: Wed, 8 Mar 2023 18:47:26 -0800 Subject: [PATCH 2/2] Add a test for removing noop --- .bazelrc | 4 +- .../src/executor/graph_executor_test.ts | 63 +++++++++++ .../src/operations/executors/utils.ts | 4 +- tfjs/yarn.lock | 106 ++---------------- 4 files changed, 74 insertions(+), 103 deletions(-) diff --git a/.bazelrc b/.bazelrc index 3bc195dc35b..ef7e13907f2 100644 --- a/.bazelrc +++ b/.bazelrc @@ -79,8 +79,8 @@ test:macos --define DISPLAY=true test:windows --define DISPLAY=true --//:headless=false # Enable debugging tests with --config=debug -run:debug --test_arg=--node_options=--inspect-brk --test_output=streamed --test_strategy=exclusive --test_timeout=999999 --nocache_test_results -test:debug --test_arg=--node_options=--inspect-brk --test_output=streamed --test_strategy=exclusive --test_timeout=999999 --nocache_test_results +run:debug --test_arg=--node_options=--inspect-brk --test_output=streamed --test_strategy=exclusive --test_timeout=36000 --nocache_test_results +test:debug --test_arg=--node_options=--inspect-brk --test_output=streamed --test_strategy=exclusive --test_timeout=36000 --nocache_test_results test:debugpy --test_output=streamed --test_strategy=exclusive --test_timeout=999999 --nocache_test_results run:debugpy --test_output=streamed --test_strategy=exclusive --test_timeout=999999 --nocache_test_results diff --git a/tfjs-converter/src/executor/graph_executor_test.ts b/tfjs-converter/src/executor/graph_executor_test.ts index e53c86e46e3..65e80d6ccc1 100644 --- a/tfjs-converter/src/executor/graph_executor_test.ts +++ b/tfjs-converter/src/executor/graph_executor_test.ts @@ -17,6 +17,7 @@ import * as tfc from '@tensorflow/tfjs-core'; import * as tensorflow from '../data/compiled_api'; +import {createNumberAttr} from '../operations/executors/test_helper'; import {createTensorAttr} from '../operations/executors/test_helper'; import {Graph, Node} from '../operations/types'; @@ -181,6 +182,68 @@ describe('GraphExecutor', () => { tfc.test_util.expectArraysClose(await result[0].data(), [3.0]); }); + it('should skip noop', async () => { + const inputNode: Node = { + inputNames: [], + inputs: [], + children: [], + name: 'input', + op: 'Placeholder', + category: 'graph', + attrParams: {}, + inputParams: {} + }; + + const noopNode: Node = { + inputNames: ['input'], + inputs: [inputNode], + children: [], + name: 'noop', + op: 'NoOp', + category: 'graph', + inputParams: {}, + attrParams: {} + }; + + const packNode: Node = { + // Even though `noop` is an input, it should be excluded during + // execution + inputNames: ['input', 'noop'], + inputs: [inputNode, noopNode], + children: [], + name: 'pack', + op: 'Pack', + category: 'slice_join', + inputParams: { + tensors: { // this range matches all the tensors in the input + 'type': 'tensors', + 'inputIndexStart': 0, + 'inputIndexEnd': 0, + } + }, + attrParams: {axis: createNumberAttr(0)} + }; + inputNode.children.push(noopNode, packNode); + noopNode.children.push(packNode); + + const graph: Graph = { + inputs: [inputNode], + nodes: { + 'input': inputNode, + 'noop': noopNode, + 'pack': packNode, + }, + outputs: [packNode], + placeholders: [inputNode], + weights: [] + }; + + const executor = new GraphExecutor(graph); + const inputTensor = tfc.tensor1d([123, 456]); + const result = executor.execute({input: inputTensor}, ['pack']); + tfc.test_util.expectArraysClose(await result[0].data(), [123, 456]); + }); + describe('strict input check', () => { it('should throw exception if missing inputs', () => { expect(() => executor.execute({}, ['output'])) diff --git a/tfjs-converter/src/operations/executors/utils.ts b/tfjs-converter/src/operations/executors/utils.ts index ee9157586f8..e206893becc 100644 --- a/tfjs-converter/src/operations/executors/utils.ts +++ b/tfjs-converter/src/operations/executors/utils.ts @@ -44,8 +44,8 @@ export function getParamValue( // during execution. Perhaps have different sets of children, one for // control dependencies and another for real dependencies. const inputs = node.inputs.slice(start, end); - const inputNames = inputs.filter(node => node.op !== 'NoOp') - .map(node => node.name); + const inputNames = node.inputNames.slice(start, end) + .filter((_name, index) => inputs[index]?.op !== 'NoOp'); return inputNames.map( name => getTensor(name, tensorMap, context, resourceManager)); diff --git a/tfjs/yarn.lock b/tfjs/yarn.lock index 3d76bce290e..e41e0948d88 100644 --- a/tfjs/yarn.lock +++ b/tfjs/yarn.lock @@ -1760,9 +1760,6 @@ resolved "https://registry.yarnpkg.com/@socket.io/component-emitter/-/component-emitter-3.1.0.tgz#96116f2a912e0c02817345b3c10751069920d553" integrity sha512-+9jVqKhRSpsc591z5vX+X5Yyw+he/HCB4iQ/RYxw35CEPaY1gnsNE43nf9n9AaYjAQrTiI/mOwKUKdUs9vf7Xg== -"@tensorflow/tfjs-backend-cpu@link:../link-package/node_modules/@tensorflow/link-package/node_modules/@tensorflow/tfjs-backend-cpu": - version "0.0.0" - "@tensorflow/tfjs-backend-cpu@link:../link-package/node_modules/@tensorflow/tfjs-backend-cpu": version "0.0.0" uid "" @@ -1825,24 +1822,11 @@ resolved "https://registry.yarnpkg.com/@types/jasmine/-/jasmine-2.8.7.tgz#3fe583928ae0a22cdd34cedf930eeffeda2602fd" integrity sha512-RdbrPcW1aD78UmdLiDa9ZCKrbR5Go8PXh6GCpb4oIOkWVEusubSJJDrP4c5RYOu8m/CBz+ygZpicj6Pgms5a4Q== -"@types/long@^4.0.1": - version "4.0.2" - resolved "https://registry.yarnpkg.com/@types/long/-/long-4.0.2.tgz#b74129719fc8d11c01868010082d483b7545591a" - integrity sha512-MqTGEo5bj5t157U6fA/BiDynNkn0YknVdh48CMPkTSpFTVmvao5UQmm7uEF6xBEo7qIMAlY/JSleYaE6VOdpaA== - "@types/minimatch@*": version "3.0.4" resolved "https://registry.yarnpkg.com/@types/minimatch/-/minimatch-3.0.4.tgz#f0ec25dbf2f0e4b18647313ac031134ca5b24b21" integrity sha512-1z8k4wzFnNjVK/tlxvrWuK5WMt6mydWWP7+zvH5eFep4oj+UkrfiJTRtjCeBXNpwaA/FYqqtb4/QS4ianFpIRA== -"@types/node-fetch@^2.1.2": - version "2.6.2" - resolved "https://registry.yarnpkg.com/@types/node-fetch/-/node-fetch-2.6.2.tgz#d1a9c5fd049d9415dce61571557104dec3ec81da" - integrity sha512-DHqhlq5jeESLy19TYhLakJ07kNumXWjcDdxXsLUMJZ6ue8VZJj4kLPQVE/2mdHh3xZziNF1xppu5lwmS53HR+A== - dependencies: - "@types/node" "*" - form-data "^3.0.0" - "@types/node@*", "@types/node@>=10.0.0": version "18.11.9" resolved "https://registry.yarnpkg.com/@types/node/-/node-18.11.9.tgz#02d013de7058cea16d36168ef2fc653464cfbad4" @@ -1853,16 +1837,6 @@ resolved "https://registry.yarnpkg.com/@types/node/-/node-10.17.56.tgz#010c9e047c3ff09ddcd11cbb6cf5912725cdc2b3" integrity sha512-LuAa6t1t0Bfw4CuSR0UITsm1hP17YL+u82kfHGrHUWdhlBtH7sa7jGY5z7glGaIj/WDYDkRtgGd+KCjCzxBW1w== -"@types/offscreencanvas@~2019.3.0": - version "2019.3.0" - resolved "https://registry.yarnpkg.com/@types/offscreencanvas/-/offscreencanvas-2019.3.0.tgz#3336428ec7e9180cf4566dfea5da04eb586a6553" - integrity sha512-esIJx9bQg+QYF0ra8GnvfianIY8qWB0GBx54PK5Eps6m+xTj86KLavHv6qDhzKcu5UUOgNfJ2pWaIIV7TRUd9Q== - -"@types/offscreencanvas@~2019.7.0": - version "2019.7.0" - resolved "https://registry.yarnpkg.com/@types/offscreencanvas/-/offscreencanvas-2019.7.0.tgz#e4a932069db47bb3eabeb0b305502d01586fa90d" - integrity sha512-PGcyveRIpL1XIqK8eBsmRBt76eFgtzuPiSTyKHZxnGemp2yzGzWpjYKAfK3wIMiU7eH+851yEpiuP8JZerTmWg== - "@types/resolve@0.0.8": version "0.0.8" resolved "https://registry.yarnpkg.com/@types/resolve/-/resolve-0.0.8.tgz#f26074d238e02659e323ce1a13d041eee280e194" @@ -1883,11 +1857,6 @@ "@types/glob" "*" "@types/node" "*" -"@types/webgl-ext@0.0.30": - version "0.0.30" - resolved "https://registry.yarnpkg.com/@types/webgl-ext/-/webgl-ext-0.0.30.tgz#0ce498c16a41a23d15289e0b844d945b25f0fb9d" - integrity sha512-LKVgNmBxN0BbljJrVUwkxwRYqzsAEPcZOe6S2T6ZaBDIrFp0qu4FNlpc5sM1tGbXUYFgdVQIoeLk1Y1UoblyEg== - "@types/yargs-parser@*": version "20.2.0" resolved "https://registry.yarnpkg.com/@types/yargs-parser/-/yargs-parser-20.2.0.tgz#dd3e6699ba3237f0348cd085e4698780204842f9" @@ -1900,11 +1869,6 @@ dependencies: "@types/yargs-parser" "*" -"@webgpu/types@0.1.21": - version "0.1.21" - resolved "https://registry.yarnpkg.com/@webgpu/types/-/types-0.1.21.tgz#b181202daec30d66ccd67264de23814cfd176d3a" - integrity sha512-pUrWq3V5PiSGFLeLxoGqReTZmiiXwY3jRkIG5sLLKjyqNxrwm/04b4nw7LSmGWJcKk59XOM/YRTUwOzo4MMlow== - accepts@~1.3.4: version "1.3.8" resolved "https://registry.yarnpkg.com/accepts/-/accepts-1.3.8.tgz#0bf0be125b67014adcb0b0921e62db7bffe16b2e" @@ -2019,11 +1983,6 @@ async@^3.0.1: resolved "https://registry.yarnpkg.com/async/-/async-3.2.0.tgz#b3a2685c5ebb641d3de02d161002c60fc9f85720" integrity sha512-TR2mEZFVOj2pLStYxLht7TyfuRzaydfpxr3k9RpHIzMgw7A64dzsdqCxH1WJyQdoe8T10nDXd9wnEigmiuHIZw== -asynckit@^0.4.0: - version "0.4.0" - resolved "https://registry.yarnpkg.com/asynckit/-/asynckit-0.4.0.tgz#c79ed97f7f34cb8f2ba1bc9790bcc366474b4b79" - integrity sha512-Oei9OH4tRh0YqU3GxhX79dM/mwVgvbZJaSNaRk+bshkj0S5cfHcgYakreBjrHwatXKbz+IoIdYLxrKim2MjW0Q== - available-typed-arrays@^1.0.2: version "1.0.2" resolved "https://registry.yarnpkg.com/available-typed-arrays/-/available-typed-arrays-1.0.2.tgz#6b098ca9d8039079ee3f77f7b783c4480ba513f5" @@ -2436,13 +2395,6 @@ combine-source-map@^0.8.0: lodash.memoize "~3.0.3" source-map "~0.5.3" -combined-stream@^1.0.8: - version "1.0.8" - resolved "https://registry.yarnpkg.com/combined-stream/-/combined-stream-1.0.8.tgz#c3d45a8b34fd730631a110a8a2520682b31d5a7f" - integrity sha512-FQN4MRfuJeHf7cBbBMJFXhKSDq+2kAArBlmRBvcvFE5BB1HZKXtSFASDhdlz9zOYwxh8lDdnvmMOe/+5cdoEdg== - dependencies: - delayed-stream "~1.0.0" - commander@^2.12.1, commander@^2.20.0: version "2.20.3" resolved "https://registry.yarnpkg.com/commander/-/commander-2.20.3.tgz#fd485e84c03eb4881c20722ba48035e8531aeb33" @@ -2642,11 +2594,6 @@ define-properties@^1.1.3: dependencies: object-keys "^1.0.12" -delayed-stream@~1.0.0: - version "1.0.0" - resolved "https://registry.yarnpkg.com/delayed-stream/-/delayed-stream-1.0.0.tgz#df3ae199acadfb7d440aaae0b29e2272b24ec619" - integrity sha512-ZySD7Nf91aLB0RxL4KGrKHBXl7Eds1DAmEdcoVawXnLD7SDhpNgtuII2aAkg7a7QS41jxPSZ17p4VdGnMHk3MQ== - depd@~1.1.2: version "1.1.2" resolved "https://registry.yarnpkg.com/depd/-/depd-1.1.2.tgz#9bcd52e14c097763e749b274c4346ed2e560b5a9" @@ -2934,15 +2881,6 @@ foreach@^2.0.5: resolved "https://registry.yarnpkg.com/foreach/-/foreach-2.0.5.tgz#0bee005018aeb260d0a3af3ae658dd0136ec1b99" integrity sha1-C+4AUBiusmDQo6865ljdATbsG5k= -form-data@^3.0.0: - version "3.0.1" - resolved "https://registry.yarnpkg.com/form-data/-/form-data-3.0.1.tgz#ebd53791b78356a99af9a300d4282c4d5eb9755f" - integrity sha512-RHkBKtLWUVwd7SqRIvCZMEvAMoGUp0XU+seQiZejj0COz3RI3hWP4sCv3gZWWLjJTd7rGwcsF5eKZGii0r/hbg== - dependencies: - asynckit "^0.4.0" - combined-stream "^1.0.8" - mime-types "^2.1.12" - from@~0: version "0.1.7" resolved "https://registry.yarnpkg.com/from/-/from-0.1.7.tgz#83c60afc58b9c56997007ed1a768b3ab303a44fe" @@ -3669,11 +3607,6 @@ log4js@^6.3.0, log4js@^6.4.1: rfdc "^1.3.0" streamroller "^3.0.2" -long@4.0.0: - version "4.0.0" - resolved "https://registry.yarnpkg.com/long/-/long-4.0.0.tgz#9a7b71cfb7d361a194ea555241c92f7468d5bf28" - integrity sha512-XsP+KhQif4bjX1kbuSiySJFNAehNxgLb6hPRGJ9QsUr8ajHkuXGdrHmFUTUUXhDwVX2R5bY4JNZEwbUiMhV+MA== - magic-string@^0.25.2: version "0.25.9" resolved "https://registry.yarnpkg.com/magic-string/-/magic-string-0.25.9.tgz#de7f9faf91ef8a1c91d02c2e5314c8277dbcdd1c" @@ -3747,13 +3680,6 @@ mime-db@1.52.0: resolved "https://registry.yarnpkg.com/mime-db/-/mime-db-1.52.0.tgz#bbabcdc02859f4987301c856e3387ce5ec43bf70" integrity sha512-sPU4uV7dYlvtWJxwwxHD0PuihVNiE7TyAbQ5SWxDCB9mUYvOgroQOwYQQOKPJ8CIbE+1ETVlOoK1UC2nU3gYvg== -mime-types@^2.1.12, mime-types@~2.1.34: - version "2.1.35" - resolved "https://registry.yarnpkg.com/mime-types/-/mime-types-2.1.35.tgz#381a871b62a734450660ae3deee44813f70d959a" - integrity sha512-ZDY+bPm5zTTF+YpCrAU9nK0UgICYPT0QtT1NZWFv4s++TNkcgVaT0g6+4R2uI4MjQjzysHB1zxuWL50hzaeXiw== - dependencies: - mime-db "1.52.0" - mime-types@~2.1.24: version "2.1.34" resolved "https://registry.yarnpkg.com/mime-types/-/mime-types-2.1.34.tgz#5a712f9ec1503511a945803640fafe09d3793c24" @@ -3761,6 +3687,13 @@ mime-types@~2.1.24: dependencies: mime-db "1.51.0" +mime-types@~2.1.34: + version "2.1.35" + resolved "https://registry.yarnpkg.com/mime-types/-/mime-types-2.1.35.tgz#381a871b62a734450660ae3deee44813f70d959a" + integrity sha512-ZDY+bPm5zTTF+YpCrAU9nK0UgICYPT0QtT1NZWFv4s++TNkcgVaT0g6+4R2uI4MjQjzysHB1zxuWL50hzaeXiw== + dependencies: + mime-db "1.52.0" + mime@^2.5.2: version "2.6.0" resolved "https://registry.yarnpkg.com/mime/-/mime-2.6.0.tgz#a2a682a95cd4d0cb1d6257e28f83da7e35800367" @@ -3825,13 +3758,6 @@ nice-try@^1.0.4: resolved "https://registry.yarnpkg.com/nice-try/-/nice-try-1.0.5.tgz#a3378a7696ce7d223e88fc9b764bd7ef1089e366" integrity sha512-1nh45deeb5olNY7eX82BkPO7SSxR5SSYJiPTrTdFUVYwAl8CKMA5N9PjTYkHiRjisVcxcQ1HXdLhx2qxxJzLNQ== -node-fetch@~2.6.1: - version "2.6.8" - resolved "https://registry.yarnpkg.com/node-fetch/-/node-fetch-2.6.8.tgz#a68d30b162bc1d8fd71a367e81b997e1f4d4937e" - integrity sha512-RZ6dBYuj8dRSfxpUSu+NsdF1dpPpluJxwOp+6IoDp/sH2QNDSvurYsAa+F1WxY2RjA1iP93xhcsUoYbF2XBqVg== - dependencies: - whatwg-url "^5.0.0" - node-releases@^1.1.71: version "1.1.72" resolved "https://registry.yarnpkg.com/node-releases/-/node-releases-1.1.72.tgz#14802ab6b1039a79a0c7d662b610a5bbd76eacbe" @@ -4722,11 +4648,6 @@ toidentifier@1.0.0: resolved "https://registry.yarnpkg.com/toidentifier/-/toidentifier-1.0.0.tgz#7e1be3470f1e77948bc43d94a3c8f4d7752ba553" integrity sha512-yaOH/Pk/VEhBWWTlhI+qXxDFXlejDGcQipMlyxda9nthulaxLZUNcUqFxokp0vcYnvteJln5FNQDRrxj3YcbVw== -tr46@~0.0.3: - version "0.0.3" - resolved "https://registry.yarnpkg.com/tr46/-/tr46-0.0.3.tgz#8184fd347dac9cdc185992f3a6622e14b9d9ab6a" - integrity sha512-N3WMsuqV66lT30CrXNbEjx4GEwlow3v6rr4mCcv6prnfwhS01rkgyFdjPNBYd9br7LpXV1+Emh01fHnq2Gdgrw== - ts-node@~8.8.2: version "8.8.2" resolved "https://registry.yarnpkg.com/ts-node/-/ts-node-8.8.2.tgz#0b39e690bee39ea5111513a9d2bcdc0bc121755f" @@ -4935,19 +4856,6 @@ wcwidth@^1.0.1: dependencies: defaults "^1.0.3" -webidl-conversions@^3.0.0: - version "3.0.1" - resolved "https://registry.yarnpkg.com/webidl-conversions/-/webidl-conversions-3.0.1.tgz#24534275e2a7bc6be7bc86611cc16ae0a5654871" - integrity sha512-2JAn3z8AR6rjK8Sm8orRC0h/bcl/DqL7tRPdGZ4I1CjdF+EaMLmYxBHyXuKL849eucPFhvBoxMsflfOb8kxaeQ== - -whatwg-url@^5.0.0: - version "5.0.0" - resolved "https://registry.yarnpkg.com/whatwg-url/-/whatwg-url-5.0.0.tgz#966454e8765462e37644d3626f6742ce8b70965d" - integrity sha512-saE57nupxk6v3HY35+jzBwYa0rKSy0XR8JSxZPwgLr7ys0IBzhGviA1/TUGJLmSVqs8pb9AnvICXEuOHLprYTw== - dependencies: - tr46 "~0.0.3" - webidl-conversions "^3.0.0" - which-boxed-primitive@^1.0.2: version "1.0.2" resolved "https://registry.yarnpkg.com/which-boxed-primitive/-/which-boxed-primitive-1.0.2.tgz#13757bc89b209b049fe5d86430e21cf40a89a8e6"