feat: For QDQ models, disable constant folding and ensure the conv weights are transposed without using an explicit op #1839
Conversation
Signed-off-by: Dheeraj Peri <[email protected]>
peri044
changed the title
Signed-off-by: Dheeraj Peri <[email protected]>
tf2onnx/tf_loader.py
Outdated
| ] | ||
| # This flag disables folding QDQ nodes around constants in the network (eg: around conv/FC weights) | ||
| rewrite_options.experimental_disable_folding_quantization_emulation = True |
There was a problem hiding this comment.
This does not exist in tensorflow v1.x versions which is also need to be supported by us.
Please add a validation(is_tf2()) here to make sure this will only be called when tf version is 2.x so the CI could pass.
There was a problem hiding this comment.
Thank you @fatcat-z Added it now.
Signed-off-by: Dheeraj Peri <[email protected]>
tf2onnx/onnx_opset/nn.py
Outdated
| val = np.transpose(val, permutation) | ||
| weights_node.set_tensor_value(val) | ||
| need_transpose = False | ||
| #Change the quantization axis for Q and DQ node accordingly |
There was a problem hiding this comment.
Please update it to "# Change ..." for the format.
There was a problem hiding this comment.
@fatcat-z Done. There are some tflite errors which are unrelated to my changes. Are those supposed to pass ?
There was a problem hiding this comment.
@fatcat-z Done. There are some tflite errors which are unrelated to my changes. Are those supposed to pass ?
Those tflite errors are not relative to your changes, they are caused by the latest tensorflow version, so you can ignore them.
There was a problem hiding this comment.
hm, the tensorflow version did not change so this must be something else that changed.
While the tflite fails are unrelated to this PR, somebody still need to fix it since CI won't let us merge this to master if it breaks the build.
Signed-off-by: Dheeraj Peri <[email protected]>
Signed-off-by: Dheeraj Peri <[email protected]>
|
This pull request introduces 1 alert and fixes 3 when merging acb77f4 into 0da3c78 - view on LGTM.com new alerts:
fixed alerts:
|
tf2onnx/tf_loader.py
Outdated
| @@ -684,8 +684,12 @@ def tf_optimize_grappler(input_names, output_names, graph_def, fold_constant=Non | |||
| # depends on so for now don't turn this on, fold_constant is always enabled now. | |||
| rewrite_options.optimizers[:] = [ | |||
| # 'pruning', 'constfold', 'arithmetic', 'dependency', 'function', | |||
| 'constfold', 'function' | |||
| 'function', 'dependency' | |||
There was a problem hiding this comment.
Why do you remove constfold optimizer for all scenarios here? I believe we only want to disable it for QDQ models, right?
Probably we should bring it back and add one more judgement below besides the version.
Signed-off-by: Dheeraj Peri <[email protected]>
Signed-off-by: Dheeraj Peri <[email protected]>
'optimizers' variable is in 'optimizer.optimize_graph' in '_convert_common'
Fix concat issue by enabling access to the 'optimizers' variable
|
Gwena - Workstation seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
'optimizers' variable is in 'optimizer.optimize_graph' in '_convert_common' Signed-off-by: Gwena - Workstation <[email protected]>
Email change
|
It seems the last issue is with my username (it's |
There is no too much experience for such case yet. Is it possible you can sync with @peri044 to create a new PR with all latest changes of this one and submitted by @peri044 so that your user name won't bother any more? |
For QDQ models, disable constant folding and ensure the conv weights are transposed without using an explicit op