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.

Sign up for GitHub

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

Jump to bottom

use tf.shape instead of .shape for dynamic axes in InstanceNormalization #771

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

use tf.shape instead of .shape for dynamic axes in InstanceNormalization #771

wants to merge 12 commits into from

Conversation

masakistan
Copy link

InstanceNormalization failed to handle tensor shape with undefined/batch size dimension. This PR fixes it by querying the dynamic shape instead of creating a constant from static shape.

Essentially the same as #543

@CLAassistant
Copy link

CLAassistant commented Sep 26, 2020 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@masakistan masakistan changed the title use tf.shape instead of .shape for dynamic axes use tf.shape instead of .shape for dynamic axes in InstanceNormalization Sep 26, 2020
@masakistan masakistan force-pushed the fix_instance_norm branch 4 times, most recently from 09bba9a to eed87e3 Compare September 26, 2020 09:54
masakistan and others added 2 commits September 26, 2020 04:21
@masakistan
use tf.shape instead of .shape for dynamic axes
3bc773c 
Signed-off-by: masakistan <[email protected]>
@chinhuang007 @masakistan
Add model stepping test for Mnist (onnx#734)
7b27f5d 
* Add model stepping test for Mnist

Add model stepping test for Mnist using ONNX runtime. The
assumption is that ONNX runtime is installed and the mnist model
from ONNX model zoo is downloaded.

Signed-off-by: Chin Huang <[email protected]>

* add tensor_dict back in TFRep

Signed-off-by: Chin Huang <[email protected]>
winnietsang
winnietsang requested changes Nov 7, 2020
View reviewed changes
Copy link
Collaborator

@winnietsang winnietsang left a comment

Choose a reason for hiding this comment

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

@masakistan Thanks for helping us to identify and fix this dynamic shape issue.
May you please add in a dynamic shape testcase for instance_normalization into onnx-tensorflow/test/backend/test_dynamic_shape.py

onnx_tf/handlers/backend/instance_normalization.py Outdated Show resolved Hide resolved
@masakistan masakistan force-pushed the fix_instance_norm branch 2 times, most recently from 7bab839 to 610feae Compare November 12, 2020 22:09
@winnietsang
Copy link
Collaborator

@masakistan please add in a dynamic shape testcase for instance_normalization into onnx-tensorflow/test/backend/test_dynamic_shape.py

@masakistan
Copy link
Author

@masakistan please add in a dynamic shape testcase for instance_normalization into onnx-tensorflow/test/backend/test_dynamic_shape.py

Will do, sorry I haven't had time to get to it.

@d-miketa d-miketa mentioned this pull request Mar 15, 2021
Merged
@mohamadHN93
Copy link

@masakistan please add in a dynamic shape testcase for instance_normalization into onnx-tensorflow/test/backend/test_dynamic_shape.py

Hi, I want to convert my onnx file to the tensorflow considering dynamic batch size, but it fails after lots of trial, could you help me
I've attached the onnx file here
https://drive.google.com/file/d/15m7fXX7Q2M5EMVLxbiqeUktKbjZPdlST/view?usp=sharing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@winnietsang winnietsang winnietsang requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@masakistan @CLAassistant @winnietsang @mohamadHN93 @chinhuang007