Add TF-Op to tfjs-op mapper to converter

[tafsiri]

This PR introduces a tool to map from TF-Ops (aka TF-kernels) used in the converter/model.json to the corresponding tfjs op the converter calls. It does this by parsing the executor code.

All TF-ops that do not map into a tfjs-op are listed in a test file to check that only those ops have empty mappings. This includes ones we expect (like TensorListGetItem) as well as a few that should map to ops but don't currently because they used chained ops (e.g. Placeholder). A followup PR will remove chained op calls from converter (similar to #3706). With that the list of unmapped ops will shrink.

This tool is run on build/test and the result is checked into the repo (metadata/kernel2op.json) so that we can have a mapping specific to each commit (and more importantly each release) of tfjs.

We need this mapping to generate custom op imports for the converter based on a specific model.json.

To see the logs from the Cloud Build CI, please join either our discussion or announcement mailing list.

---

This change is 

[pyu10055]

Thank you for continue driving the modularization work. An high level question, this seems to require the converter executor code to follow the pattern of the parser? Can there be a test that warns on unparsible code?

Reviewable status: 0 of 1 approvals obtained (waiting on @annxingyuan, @lina128, @pyu10055, and @tafsiri)

---

tfjs-converter/metadata/kernel2op.json, line 1 at r1 (raw file):

{

should this file be an ts file, so it can be included in the source easier?
The benefit is in the future this mapping can be the source of truth and simplify and drive the executor code to call tfc programatically.

---

tfjs-converter/scripts/kernels_to_ops.ts, line 3 at r1 (raw file):

/**
 * @license
 * Copyright 2020 Google Inc. All Rights Reserved.

Google LLC

---

tfjs-converter/src/metadata_test.ts, line 21 at r1 (raw file):

// we know we don't map to tfjs ops have empty entries in metadata
// kernel2op.json
describe('kernel2op metadata file', () => {

is it possible to validate the mapping? like testing the mapped method actually exists?

[pyu10055]

Reviewed 9 of 9 files at r1.
Reviewable status: 0 of 1 approvals obtained (waiting on @annxingyuan, @lina128, @pyu10055, and @tafsiri)

[tafsiri]

The test that would catch that tests to see if we generate no mapping for an Op where we would expect to see one. Then we know the parsing has failed. So if the code changes and it can't extract a function call for an op it used to (and that would be visible in the PR diff) that test will fail.

Other than that the parser tries not to place too much requirements on the code (it just tried to follow existing convention)—and if it does we can change the parser to capture any new pattern we really want to introduce. At the moment it just wants the all tfjs ops used for a Kernel to be inside the case block for that kernel, if more are present it will still work it may just import some unnecessary ops.

Reviewable status: 0 of 1 approvals obtained (waiting on @annxingyuan, @lina128, and @pyu10055)

---

tfjs-converter/metadata/kernel2op.json, line 1 at r1 (raw file):

Previously, pyu10055 (Ping Yu) wrote…

should this file be an ts file, so it can be included in the source easier?
The benefit is in the future this mapping can be the source of truth and simplify and drive the executor code to call tfc programatically.

I don't know if this can be the source of truth since it tries to rely on scanning the executor code as the actual source of truth of what gets called. It also tries to capture things like tidy and other tfjs api calls that are used but may not be needed for kernel execution. I think this could in future help with generating the supported ops table in the docs though.

I guess in future if we have a lot of confidence in this we could turn it into a hand maintained list that the source code uses? But i'm not sure how that would work/stay in sync. But when that happens we can easily change this at that time.

---

tfjs-converter/scripts/kernels_to_ops.ts, line 3 at r1 (raw file):

Previously, pyu10055 (Ping Yu) wrote…

Google LLC

Done

---

tfjs-converter/src/metadata_test.ts, line 21 at r1 (raw file):

Previously, pyu10055 (Ping Yu) wrote…

is it possible to validate the mapping? like testing the mapped method actually exists?

In a future step we will be using this file to actually generate imports based on this mapping and I plan to do validation there (along with imports generated for other elements that go into a custom build). Because the mapping from namespace to file path it not trivial (depends on core's file organization and build scripts), I want to centralize that with the custom build tooling. Along with the e2e tests for should give good coverage of the mapping.

As long as we can parse the methods calls correctly here, then the converter code itself helps validate that the methods actually exist (e.g. if a method gets renamed or removed, then the converter won't compile and when it does the file should be regenerated). So I think those two things together will give us confidence that a generated custom build is valid.

But I'll continue to keep an eye on that as the pieces come together.

[pyu10055]

Reviewable status: complete! 1 of 1 approvals obtained (waiting on @annxingyuan, @lina128, @pyu10055, and @tafsiri)

---

tfjs-converter/metadata/kernel2op.json, line 1 at r1 (raw file):

Previously, tafsiri (Yannick Assogba) wrote…

I don't know if this can be the source of truth since it tries to rely on scanning the executor code as the actual source of truth of what gets called. It also tries to capture things like tidy and other tfjs api calls that are used but may not be needed for kernel execution. I think this could in future help with generating the supported ops table in the docs though.

I guess in future if we have a lot of confidence in this we could turn it into a hand maintained list that the source code uses? But i'm not sure how that would work/stay in sync. But when that happens we can easily change this at that time.

cool, can we make sure this file is not included in the tfjs-converter npm?

[tafsiri]

@pyu10055 we need this file to be in the npm package so that the build tool can read it when parsing model.json files. We don't want the end user to need to check out the repo.