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

Fix unsupported ops TF 2.14.0: OnesLike #2270

Merged
merged 5 commits into from
Nov 16, 2023
Merged

Fix unsupported ops TF 2.14.0: OnesLike #2270

merged 5 commits into from
Nov 16, 2023

Conversation

Gerstenberger
Copy link
Contributor

Fixes #2267
Do you want a test case for this?

@fatcat-z
Copy link
Collaborator

Thanks for your contributions!

Yes, please add a test case as well.

Signed-off-by: Alexander Gerstenberger <agerstenberger@apptek.com>
Signed-off-by: Alexander Gerstenberger <agerstenberger@apptek.com>
@Gerstenberger
Copy link
Contributor Author

Added some test cases

Signed-off-by: Alexander Gerstenberger <agerstenberger@apptek.com>
@@ -4105,6 +4105,54 @@ def func(x, y):
self._run_test_case(func, [_OUTPUT], {_INPUT: input_x.astype(np.int32), _INPUT1: input_y}, as_session=True,
graph_validator=lambda g: check_op_count(g, "ConstantOfShape", 1, disabled=False))

@check_tf_max_version("2.13.1")
Copy link
Collaborator

Choose a reason for hiding this comment

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

This means this test case won't be run on any versions above 2.13.1. Could you please share why we can't do this? Or we can remove this limitation?

@@ -4105,6 +4105,54 @@ def func(x, y):
self._run_test_case(func, [_OUTPUT], {_INPUT: input_x.astype(np.int32), _INPUT1: input_y}, as_session=True,
graph_validator=lambda g: check_op_count(g, "ConstantOfShape", 1, disabled=False))

@check_tf_max_version("2.13.1")
@check_opset_min_version(9, "ConstantOfShape")
@check_opset_max_version(9, "ConstantOfShape")
Copy link
Collaborator

@fatcat-z fatcat-z Nov 15, 2023

Choose a reason for hiding this comment

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

Since we already have 2 functions for different versions, we don't need to add an opset limitation for these test methods. And we don't need to create different test cases for different versions.

You just need to create one test case for tf op OnesLike, and the test framework will help on testing different ops. At least, please remove the check_opset_max_version so we don't limit it running on higher opset version which supports ConstantOfShape.

self._run_test_case(func, [_OUTPUT], {_INPUT: input_x.astype(np.int32), _INPUT1: input_y}, as_session=True,
graph_validator=lambda g: check_op_count(g, "Expand", 1, disabled=False))

@check_tf_min_version("2.14.0")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we need to require 2.14.0 version to run this test case?

Signed-off-by: Alexander Gerstenberger <agerstenberger@apptek.com>
@Gerstenberger
Copy link
Contributor Author

Hi, thanks for your comments.
I only kept one test case now without checking for specific ops (i think that more natural in this case).

Copy link
Collaborator

@fatcat-z fatcat-z left a comment

Choose a reason for hiding this comment

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

Thanks for your contributions!

@fatcat-z fatcat-z merged commit ae4c39e into onnx:main Nov 16, 2023
51 checks passed
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.

Unsupported ops TF 2.14.0: OnesLike not supported
2 participants