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

[Feature request] Extend shape inference tests to cover op version <= 5 #5289

Open
guoyuhong opened this issue Jun 6, 2023 · 3 comments · May be fixed by #5354
Open

[Feature request] Extend shape inference tests to cover op version <= 5 #5289

guoyuhong opened this issue Jun 6, 2023 · 3 comments · May be fixed by #5354
Labels
enhancement Request for new feature or operator

Comments

@guoyuhong
Copy link
Contributor

System information

main branch

What is the problem that this feature solves?

I'm working on Issue #4160 through PR #5263 .
Current design works fine for Op version > 5. For Op version <= 5. I found the following error:
image
I looks like the Reshape creation arguments do not fit for Op version <= 5.

This issue is used to track this problem in case it is forgotten.

Alternatives considered

The code change for PR #5263 could be big, so this issue will be solve later on.

Describe the feature

No response

Will this influence the current api (Y/N)?

No response

Feature Area

No response

Are you willing to contribute it (Y/N)

Yes

Notes

No response

@guoyuhong guoyuhong added the enhancement Request for new feature or operator label Jun 6, 2023
@gramalingam
Copy link
Contributor

I am surprised that this problem does not show up more often. There are other ops where attributes are promoted to become inputs. The test-creation logic, however, has to explicitly indicate which are inputs, which are attributes etc. Eg. the Reduce* ops.

@gramalingam
Copy link
Contributor

I think, in the end, users creating a test will probably need to specify actual versions, like in the case of Reshape. May be with other utilities like all_versions('Reshape', least_version=5)

@guoyuhong
Copy link
Contributor Author

@gramalingam The PR #5263 is trying to solve the problem to increase test coverage. Before the PR, onnx/test/shape_inference_test.py only covers the latest version of Ops. When developers introduced incompatible change, developers often deleted the old test code and only kept the latest test code. The PR introduced a scheme that requires test function to cover all versions of Ops. If incompatible changes are introduced, the developers need to add new test code and keep the old test code.

I'm now trying to solve the inference test for OpVersion <= 5. I fixed the Reshape creation problem. But I found that when OpVersion=1, the shape inference seems not inference the output shape(showed in following figure). I found that many Ops had the same problem when OpVersion==1. I don't know the root cause. Maybe some C++ code needs to be changed.
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Request for new feature or operator
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants