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 support for converting models with ImageProjectiveTransformV3 op #6206

Merged
merged 7 commits into from
Mar 15, 2022

Conversation

maciej3031
Copy link
Contributor

@maciej3031 maciej3031 commented Mar 8, 2022

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

This PR covers the following issue #6204


This change is Reviewable

@google-cla
Copy link

google-cla bot commented Mar 8, 2022

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

For more information, open the CLA check for this pull request.

@rthadur rthadur requested a review from lina128 March 8, 2022 18:30
Copy link
Collaborator

@lina128 lina128 left a comment

Choose a reason for hiding this comment

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

This is great, thank you for your contribution, @maciej3031 ! I only have some minor comments, then the PR should be ready to merge.

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


tfjs-converter/python/tensorflowjs/op_list/image.json, line 114 at r1 (raw file):

      {
        "start": 0,
        "name": "image",

images, ref: https://github.com/tensorflow/tensorflow/blob/master/tensorflow/core/ops/ops.pbtxt#L21603-L21650

Code quote:

image

tfjs-converter/python/tensorflowjs/op_list/image.json, line 124 at r1 (raw file):

      {
        "start": 2,
        "name": "outputShape",

Also add tfName: "output_shape", ref: https://github.com/tensorflow/tensorflow/blob/master/tensorflow/core/ops/ops.pbtxt#L21603-L21650

Code quote:

outputShape

tfjs-converter/python/tensorflowjs/op_list/image.json, line 129 at r1 (raw file):

      {
        "start": 3,
        "name": "fillValue",

Also add tfName: "fill_value", ref: https://github.com/tensorflow/tensorflow/blob/master/tensorflow/core/ops/ops.pbtxt#L21603-L21650

Code quote:

fillValue

tfjs-converter/src/operations/executors/image_executor.ts, line 82 at r1 (raw file):

        }
        case 'ImageProjectiveTransformV3': {
          const image =

images, it's a 4D, may take a batch of images.

Code quote:

image

@lina128 lina128 requested a review from ahmedsabie March 14, 2022 17:47
@maciej3031
Copy link
Contributor Author

Hi @lina128 , I addressed your comments except from these two:

- Also add tfName: "output_shape", ref: [...]
- Also add tfName: "fill_value", ref: [...]

I'm not very familiar with the tfjs repo but it seems to me that tfName does not exist in InputParamMapper so it cannot be used there. It's only present in AttrParamMapper 🤔

@maciej3031 maciej3031 requested a review from lina128 March 15, 2022 11:39
@lina128
Copy link
Collaborator

lina128 commented Mar 15, 2022

Ah, you are right.

Copy link
Collaborator

@lina128 lina128 left a comment

Choose a reason for hiding this comment

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

:lgtm_strong:

Reviewable status: :shipit: complete! 1 of 1 approvals obtained (waiting on @ahmedsabie and @maciej3031)

@lina128 lina128 merged commit 00d949a into tensorflow:master Mar 15, 2022
@maciej3031 maciej3031 deleted the issue-6204 branch March 15, 2022 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants