Retain qnn input kernel scales #4292
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
anijain2305
reviewed
Nov 9, 2019
Sure. When I wrote it, I felt it better to keep the name distinct to indicate the difference between input_scale in Requantize vs here but I suppose `tensor' is superfluous to requirements .. Ramana |
tqchen
added
status: need review
status: need update
|
Not sure why my testing didn't catch this - hopefully this lot satisfies the CI ... |
anijain2305
reviewed
Nov 11, 2019
anijain2305
reviewed
Nov 11, 2019
anijain2305
reviewed
Nov 11, 2019
|
All updates now done ,please review and merge as appropriate . |
|
A gentle ping for a merge. |
zhiics
reviewed
Nov 13, 2019
|
Whoops , now fixed. |
added 11 commits
November 14, 2019 23:50
kernel_tensor_scale. The lowering in the tflite frontend loses the input_tensor_scale and the kernel_tensor_scale by multiplying it and putting it into the Requantize operation. This means that any graph partitioning passes or other passes that need to access this information no longer have it available in the qnn dialect. regards Ramana
As for conv2d, the tflite frontend drops the input tensor scale and the weight tensor scale from the relay op. Store it as separate fields in there.
to kernel_scale for conv2d.
And use input_scale and kernel_scale
|
Ok, it looks like there is some more work to be done here because of other bits that have landed.
|
|
For the first problem, you can just delete the extra scale arguments from the attrs. |
added 2 commits
November 15, 2019 09:25
nn.conv2d does not contain input_scale and kernel_scale. We need to delete it when lowering it to nn.conv2d.
u99127
force-pushed
the
retain-qnn-input-kernel-scales
branch
from
80084dd to
46f3c23
Compare
Ah thanks - I hadn't spotted that last night. Now rebased and repushed. Ramana |
|
Now all done, would be nice to merge. |
|
Thanks @u99127 @anijain2305 |
zhiics
added
status: accepted
and removed
status: need review
status: need update
zxy844288792
pushed a commit
to zxy844288792/tvm
that referenced
this pull request
Nov 26, 2019
* Add qnn conv2d attributes for input_tensor_scale and kernel_tensor_scale. The lowering in the tflite frontend loses the input_tensor_scale and the kernel_tensor_scale by multiplying it and putting it into the Requantize operation. This means that any graph partitioning passes or other passes that need to access this information no longer have it available in the qnn dialect. regards Ramana * Store input tensor scale and Weight tensor scale for Dense as well As for conv2d, the tflite frontend drops the input tensor scale and the weight tensor scale from the relay op. Store it as separate fields in there. * Fix unintentional tab * Rename input_tensor_scale to input_scale and kernel_tensor_scale to kernel_scale for conv2d. * input_tensor_scale -> input_scale weight_tensor_scale->weight_scale * Rework dense testcase And use input_scale and kernel_scale * Be consistent in use of input_scale and kernel_scale values * Fixup qnn conv2d tests for input_scale and kernel_scale * Make pydoc identical between conv2d and dense for weight_tensor * Fix up conv2d parameters to be in the same order between C++ and python * Fix ordering of parameters for dense. * Add input_scale and output_scale to try and satisfy ci gods * Delete input_scale and kernel_scale. nn.conv2d does not contain input_scale and kernel_scale. We need to delete it when lowering it to nn.conv2d. * Add input_scale and kernel_scale for qnn.conv2d
yongwww
pushed a commit
to neo-ai/tvm
that referenced
this pull request
Nov 26, 2019
* Add qnn conv2d attributes for input_tensor_scale and kernel_tensor_scale. The lowering in the tflite frontend loses the input_tensor_scale and the kernel_tensor_scale by multiplying it and putting it into the Requantize operation. This means that any graph partitioning passes or other passes that need to access this information no longer have it available in the qnn dialect. regards Ramana * Store input tensor scale and Weight tensor scale for Dense as well As for conv2d, the tflite frontend drops the input tensor scale and the weight tensor scale from the relay op. Store it as separate fields in there. * Fix unintentional tab * Rename input_tensor_scale to input_scale and kernel_tensor_scale to kernel_scale for conv2d. * input_tensor_scale -> input_scale weight_tensor_scale->weight_scale * Rework dense testcase And use input_scale and kernel_scale * Be consistent in use of input_scale and kernel_scale values * Fixup qnn conv2d tests for input_scale and kernel_scale * Make pydoc identical between conv2d and dense for weight_tensor * Fix up conv2d parameters to be in the same order between C++ and python * Fix ordering of parameters for dense. * Add input_scale and output_scale to try and satisfy ci gods * Delete input_scale and kernel_scale. nn.conv2d does not contain input_scale and kernel_scale. We need to delete it when lowering it to nn.conv2d. * Add input_scale and kernel_scale for qnn.conv2d
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The QNN dialect loses the input tensor scale and the weight
tensor scale too early. This inhibits the work one could do
with integrating 3rd party codegen or libraries that have interfaces
that expect input tensor scale and weight tensor scales.
This patch stack fixes it up for conv2d and Dense. I cannot see
any other operators affected yet.
See here for more . https://discuss.tvm.ai/t/lowering-qnn-conv2d-tflite/4654/5
Ramana
@anijain2305 - Please review.