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

Support dynamic 'pads' and 'value' in Pad operator #2031

Merged
merged 107 commits into from
Sep 17, 2019

Conversation

hariharans29
Copy link
Contributor

@hariharans29 hariharans29 commented May 18, 2019

Change description:

This change is to make two static attributes of the Pad op - 'pads' and 'value' into dynamic inputs to support conversion of some TensorFlow models into ONNX which allow these 2 values to be fed in dynamically to the op. This change is to basically support conversion of such TF models into ONNX.

Summary of changes:

  • Make interface change in Pad operator as described in title
  • Rename 'value' to 'constant_value' to reflect its usage only for 'constant' mode
  • Make related node test case changes
  • Make related shape inference test changes
  • Make related changes in optimizer passes
  • Make related optimizer test changes

Issues that are resolved:
Resolves #1765 - Adds description and examples for 'Edge' and 'Reflect' mode
Resolves #2313 - Allows testing optimizer(s) for previous opset specs of ops/nodes

hariharans29 and others added 30 commits April 11, 2019 17:18
…s. (onnx#1930)

* Change the return type for the zipmap operator to match the description in the spec.

* Prevent unused variables from generating warnings across all platforms.
This was observed in onnxruntime when __ONNX_NO_DOC_STRINGS was enabled.

* nit change
* fix shape inference and add tests for shape inference

* cosmetic fixes

* plus some formatting
* Fix shape inference for matmul when there is no input shape, and provide better coverage for testcases

* add test for qlinearmatmul as weel
docs/Operators.md Outdated Show resolved Hide resolved
onnx/test/optimizer_test.py Outdated Show resolved Hide resolved
Copy link
Contributor

@ebarsoum ebarsoum left a comment

Choose a reason for hiding this comment

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

Resolve conflict, add example for other modes, and rename value to constant_value.

docs/Changelog.md Show resolved Hide resolved
docs/Changelog.md Outdated Show resolved Hide resolved
@wschin wschin merged commit 1f4b5f8 into onnx:master Sep 17, 2019
@Exlsunshine
Copy link

Hi @hariharans29, glad to see your change has been merged into master, but I still got the following error:

OP=Pad
Name=some/path/to/op/Pad
Inputs:
        some/path/to/op/concat:0=Concat, [-1, -1], 1
        some/path/to/op/Pad/paddings_Concat__113:0=Concat, [2, 2], 6
Outpus:
        some/path/to/op/Pad:0=[-1, -1], 1
Traceback (most recent call last):
  File "D:\Python\Python36\lib\site-packages\tf2onnx\tfonnx.py", line 352, in tensorflow_onnx_mapping
    func(g, node, **kwargs)
  File "D:\Python\Python36\lib\site-packages\tf2onnx\onnx_opset\nn.py", line 401, in version_1
    paddings = np.array(node.inputs[1].get_tensor_value()).transpose().flatten()
  File "D:\Python\Python36\lib\site-packages\tf2onnx\graph.py", line 256, in get_tensor_value
    raise ValueError("get tensor value: {} must be Const".format(self.name))
ValueError: get tensor value: some/path/to/op/Pad/paddings_Concat__113 must be Const
2019-11-21 15:52:39,788 - VERBOSE - tf2onnx.tfonnx: Mapping TF node to ONNX node(s)
2019-11-21 15:52:40,098 - ERROR - tf2onnx.tfonnx: Failed to convert node some/path/to/op/Fill


OP=ConstantOfShape
Name=some/path/to/op/Fill
Inputs:
        some/path/to/op/Fill__851:0=Cast, [1], 7
        some/path/to/op/strided_slice_21__842:0=Cast, [], 6
Outpus:
        some/path/to/op/Fill:0=[-1], 6
Traceback (most recent call last):
  File "D:\Python\Python36\lib\site-packages\tf2onnx\tfonnx.py", line 352, in tensorflow_onnx_mapping
    func(g, node, **kwargs)
  File "D:\Python\Python36\lib\site-packages\tf2onnx\onnx_opset\generator.py", line 100, in version_9
    value = np.array([node.inputs[1].get_tensor_value()]).astype(utils.map_onnx_to_numpy_type(dtype))
  File "D:\Python\Python36\lib\site-packages\tf2onnx\graph.py", line 256, in get_tensor_value
    raise ValueError("get tensor value: {} must be Const".format(self.name))
ValueError: get tensor value: some/path/to/op/strided_slice_21__842 must be Const

I am using:

python -m tf2onnx.convert --input a.pb --inputs in_0:0,in_1:0 --outputs o_0:0,o_1:0 --output b.onnx --opset 11 --fold_const  --continue_on_error --verbose

2019-11-21 15:52:31,785 - INFO - tf2onnx.tfonnx: Using tensorflow=1.13.1, onnx=1.6.0, tf2onnx=1.5.3/7b598d
2019-11-21 15:52:31,785 - INFO - tf2onnx.tfonnx: Using opset <onnx, 11>

Do you have any advice about this error? Thanks!

jcwchen pushed a commit to jcwchen/onnx that referenced this pull request Sep 23, 2020
* Pad spec update

* More changes

* More changes

* Make pads and value attributes inputs in Pad operator

* More changes

* More changes

* More cahnges

* More changes

* Remove tab

* Fix formatting issues

* More changes

* Pad spec update

* More changes

* More changes

* Prevent unused variables from generating warnings across all platforms.  (onnx#1930)

* Change the return type for the zipmap operator to match the description in the spec.

* Prevent unused variables from generating warnings across all platforms.
This was observed in onnxruntime when __ONNX_NO_DOC_STRINGS was enabled.

* nit change

* Shape Inference Tests for QOps (onnx#1929)

* fix shape inference and add tests for shape inference

* cosmetic fixes

* plus some formatting

* Fix shape inference for matmul (onnx#1941)

* Fix shape inference for matmul when there is no input shape, and provide better coverage for testcases

* add test for qlinearmatmul as weel

* Make pads and value attributes inputs in Pad operator

* More changes

* More changes

* More cahnges

* More changes

* Remove tab

* Fix formatting issues

* More changes

* Fix line ending issue

* Update Operators md file

* Fix typo

* Updating TestCoverage.md file

* Nit fix

* Fix accidental revert

* PR feedback

* More PR feedback changes

* Optimizer passes changes

* Remove unnecessary line in def file

* More formatting changes

* Add 2 files missed previously

* Commit missed out file

* Fix build break

* Update tests

* Initial commit

* Formatting

* Refactor ParseRawData

* More refactroing changes

* Introduce shape inference failure in case of wrong number of pad values

* Resolve comments

* Avoid vector copy

* Fix comment

* More changes

* Update Changelog.md

* Def changes

* More updates

* Formatting

* Check-in tests

* Update docs

* Update Changelog.md

* Docs update

* Update Changelog.md

* Update Changelog.md

* Resolve comments

* Add docs

* Addign docs

* Update Changelog.md

* Build break fix

* Fix build break

* Add optimizer tests for older opset Pad

* Add back older opset Pad shape inference tests

* Fix build break

* Fix build break

* Account for Value to Constant_value change in some comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
operator Issues related to ONNX operators
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow creation of older opset ops/nodes Meaning of edge and reflect in Pad operator?