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

Nano: fix keras onnx model output shape #7138

Merged

Conversation

MeouSker77
Copy link
Contributor

@MeouSker77 MeouSker77 commented Dec 30, 2022

Description

Fix the output shape of keras onnx model

1. Why the change?

tf.py_function(func, ..., Tout=tf.float32) will convert the output of func to a single Tensor, that means [t] or [[t]] (t is a tf.Tensor) all will be converted to t.

Besides, if the original model return a single tensor t, then after tracing, onnxruntime model will return [t].

These two rules will destroy the output shape when the original output is a single tensor, or a list of tensor with only one element, or a list of list of tensor with only one element, ...

Before this PR, we simply check whether the output is a list and has only one element, if is, then return its first element. We use this method to

However, this method cannot distinguish whether the original model return t or [t] (onnxruntime model will return [t] in both case). And we don't handle the influence of tf.py_function.

This PR fix both, it save the right output shape first, and convert the final output to this right shape.

2. User API changes

N/A

3. Summary of the change

N/A

4. How to test?

  • Unit test

Copy link
Contributor

@rnwang04 rnwang04 left a comment

Choose a reason for hiding this comment

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

others LGTM

python/nano/test/tf/keras/test_trace_and_quantize.py Outdated Show resolved Hide resolved
@@ -78,7 +86,10 @@ def __call__(self, *args, **kwargs):
kwargs[name] = value
for param in self._call_fn_args_backup[len(inputs):len(self._forward_args)]:
inputs.append(kwargs[param])
return self.call(*inputs)
outputs = self.call(*inputs)
Copy link
Contributor

Choose a reason for hiding this comment

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

If I understand right, here returns original onnx output(a list) ? so maybe it contains _nesting_level at first (not start from 0) ?
It's just a bit confusing whether this way of adding lists works in all cases

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, here outputs is a single Tensor, not onnx output.

Copy link
Contributor Author

@MeouSker77 MeouSker77 Jan 4, 2023

Choose a reason for hiding this comment

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

self.call will call tf.py_function(func, ..., Tout=tf.float32), which will convert the output of func to a single Tensor

Copy link
Collaborator

@TheaperDeng TheaperDeng left a comment

Choose a reason for hiding this comment

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

LGTM

@MeouSker77 MeouSker77 merged commit 0d61dc6 into intel-analytics:main Jan 4, 2023
@MeouSker77 MeouSker77 deleted the fix-keras-model-output-shape branch January 4, 2023 06:02
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.

None yet

3 participants