Skip to content

Conversation

@lina128
Copy link
Collaborator

@lina128 lina128 commented Dec 17, 2019

Tested in local with a python model.


This change is Reviewable

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 @lina128 and @pyu10055)


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

      }
    ],
    "attrs": [

There seems to be an extra attribute 'pad_to_ax_output_size', is it implemented?


tfjs-converter/src/executor/model_analysis.ts, line 133 at r1 (raw file):

const CONTROL_FLOW_OPS = ['Switch', 'Merge', 'Enter', 'Exit', 'NextIteration'];
const DYNAMIC_SHAPE_OPS = [
  'NonMaxSuppressionV2', 'NonMaxSuppressionV3', 'NonMaxSuppressionV5', 'Where'

Do we support NMS V4?

@lina128
Copy link
Collaborator Author

lina128 commented Jan 2, 2020

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

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

      }
    ],
    "attrs": [

There seems to be an extra attribute 'pad_to_ax_output_size', is it implemented?

tfjs-converter/src/executor/model_analysis.ts, line 133 at r1 (raw file):

const CONTROL_FLOW_OPS = ['Switch', 'Merge', 'Enter', 'Exit', 'NextIteration'];
const DYNAMIC_SHAPE_OPS = [
  'NonMaxSuppressionV2', 'NonMaxSuppressionV3', 'NonMaxSuppressionV5', 'Where'

Do we support NMS V4?

pad_to_ax_output_size is added in V4 and is always set to false in V5. Since we don't support V4 yet, it is safe to not expose it in API. We can start supporting V4, it will be a separate PR.

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: :shipit: complete! 1 of 1 approvals obtained

@lina128 lina128 merged commit b9a23bc into tensorflow:master Jan 2, 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.

3 participants