Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add implementation for inputs and outputs getters to SavedModel #4036

Merged
merged 9 commits into from
Oct 12, 2020

Conversation

patlevin
Copy link
Contributor

@patlevin patlevin commented Oct 7, 2020

Motivation

SavedModel currently lacks implementations for the inputs and outputs getters (see Issue #4035).

The model signature already contains all necessary information and invoking getMetaGraphsFromSavedModel() to get the information might result in redundant file I/O and -parsing.

Implementation

This implementation extracts the ModelTensorInfo from the model's signature and removes channel information from the tensor names (e.g. "output:0" is returned as just "output").


This change is Reviewable

@google-cla
Copy link

google-cla bot commented Oct 7, 2020

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added the cla: no label Oct 7, 2020
@patlevin
Copy link
Contributor Author

patlevin commented Oct 7, 2020

@googlebot I signed it!

@google-cla google-cla bot added cla: yes and removed cla: no labels Oct 7, 2020
Copy link
Collaborator

@pyu10055 pyu10055 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the contribution, overall looks great, just a minor comment. Also can you added test to these methods?

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


tfjs-node/src/saved_model.ts, line 213 at r1 (raw file):

    const results = Object.keys(entries).map((key: string) => entries[key]);
    results.forEach((info: ModelTensorInfo) => {
      info.name = info.name.replace(/:\d+$/, '');

the index number should be kept unless it is 0, it represents the which output of the node is designated for the model output.

@patlevin
Copy link
Contributor Author

patlevin commented Oct 8, 2020

@pyu10055 Thanks for the quick feedback! I added tests and changed the name-handling so indices other than zero are kept in the returned input- and output names.

Copy link
Collaborator

@pyu10055 pyu10055 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


tfjs-node/src/saved_model_test.ts, line 523 at r3 (raw file):

        './test_objects/saved_model/model_multi_output', ['serve'],
        'serving_default');
    expect(model.inputs.length).toBe(2)

missing semicolons here and below

@patlevin patlevin requested a review from pyu10055 October 10, 2020 13:16
Copy link
Collaborator

@pyu10055 pyu10055 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 2 files at r2, 1 of 1 files at r4.
Reviewable status: :shipit: complete! 1 of 1 approvals obtained (waiting on @pyu10055)

@pyu10055 pyu10055 merged commit 3c4b442 into tensorflow:master Oct 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants