[GitHub] [incubator-tvm] FrozenGene edited a comment on issue #4828: [QNN][TFLite] TFLite rounding mode support
FrozenGene edited a comment on issue #4828: [QNN][TFLite] TFLite rounding mode support URL: https://github.com/apache/incubator-tvm/pull/4828#issuecomment-596083786 > I'm fine with both choices. The pooling implementation doesn't affect any other test except for this bit-exact computation. It can be regarded as bug or feature to me : ) (as np.mean choose FLOOR implementation for integer inputs). @FrozenGene @tqchen I prefer adding it in this pr. I wish we could port it back to 0.6 as one complete pr to get bit-exact result compared with TFLite as @u99127 said before > > I think you are confusing the GPU container with GPU target. The tests are still running on CPU. > > > > * You might have done this already. But have you run mobilenetv2 locally? > > Yes, and it passed smoothly I doubt it is because of different LLVM version. Another thing: "Another way is we provide rounding args for qnn.add / qnn.mul / qnn.conconcate, because they use requantize in fact too, so they need rounding". On mobilenet v2 quantized model, we have `quantized add`, I am curious why we could get the same result without support of `quantized add`. > I have LLVM 9.0.1, compiled from source code. In my local env, `_test_forward_elemwise(partial(_test_add, fused_activation_function="RELU6"))` is failing while not in ci test. I'm not sure about the llvm version used in the ci. In code `tvm.testing.assert_allclose(tflite_output[i], tvm_output[i], atol=1e-5, rtol=1e-5)` maybe too strict. You could change it to `atol=1e-3, rtol=1e-3` to see it is ok on your machine. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-tvm] FrozenGene edited a comment on issue #4828: [QNN][TFLite] TFLite rounding mode support
FrozenGene edited a comment on issue #4828: [QNN][TFLite] TFLite rounding mode support URL: https://github.com/apache/incubator-tvm/pull/4828#issuecomment-583691425 > just modified add/mul/concat requantize rounding mode and tested, no luck. will change the default rounding behavior for a later test. > > update: I forced FixedPointMultiply(PerChannel) rounding mode to be TFLITE, but still unable to get bit-exact results. > > one more thing, setting tflite default rounding mode to TFLITE seems to break GPU test of mobilenet_v2, maybe you guys have any ideas/suggestions? > > @FrozenGene @anijain2305 Let us break the model into layer by layer and compare with tflite. I want to describe my development way, maybe it could help you. For example, we have mobilenetv2 quantized model, you could get the quantized tensorflow and tflite model. Then you could call `tflite_convert` (feed it quantized tensorflow model) and set the output layer (for example, just the first convolution layer), then you get one quantized tflite model only contain the first convolution layer of mobilenet v2. After you verify it correctly, you could go on until you finish the whole model e2e correctly. Command example: `tflite_convert --input_format=TENSORFLOW_GRAPHDEF --graph_def_file="xx.pb" --output_file=xx.tflite --output_format=TFLITE --input_arrays=input --input_shapes=1,224,224,3 --std_dev_values=127 --mean_values=127 --inference_type=QUANTIZED_UINT8 --inference_input_type=QUANTIZED_UINT8 --default_ranges_min=0 --default_ranges_max=6 --output_arrays=xx` I think when you verify, you could run on cpu firstly locally to find issue, then consider gpu ci issue. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-tvm] FrozenGene edited a comment on issue #4828: [QNN][TFLite] TFLite rounding mode support
FrozenGene edited a comment on issue #4828: [QNN][TFLite] TFLite rounding mode support URL: https://github.com/apache/incubator-tvm/pull/4828#issuecomment-583218692 > One request that I have is - While setting the rounding mode to TFLite in TFLite parser, it might be better to set it by adding an extra arg in `from_tflite` API. The main reason is that the lowering here might involve many more number of Relay ops, which might degrade performance. So, to enable TVM user to choose different accuracy-performance tradeoff points, we can add a new argument in the TFLite parser API, and then pass it along to the QNN ops. The default value of this arg can be tflite rounding. @FrozenGene Are you ok with this? Yes, this is my thought too. I am ok with it. > LGTM. I like the scope of this PR. IMO, we can get this in. And send other PRs for TFLite parser. IMO, I would prefer adding TFLite parser in this PR too. Because this will be organized into one complete solution for TFLite rounding. When others review the history, they don't need to check isolated PRs together to get complete support for TFLite rounding support. Maybe these PRs during time is long. It is right that we could make small prs and merge it in quickly so that we could get functionality supporting quickly and could locate the reason quickly. However, from this pr's perspective, TFLite's parser code will not be complex and could be contained in. It is acceptable in one single PR from my opinion. So I would suggest @LiangHao151941 adding TFLite parser in this PR too. If we think it is import, we even could port this back to release 0.6, one PR will make us do it easily too. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-tvm] FrozenGene edited a comment on issue #4828: [QNN][TFLite] TFLite rounding mode support
FrozenGene edited a comment on issue #4828: [QNN][TFLite] TFLite rounding mode support URL: https://github.com/apache/incubator-tvm/pull/4828#issuecomment-583218692 > One request that I have is - While setting the rounding mode to TFLite in TFLite parser, it might be better to set it by adding an extra arg in `from_tflite` API. The main reason is that the lowering here might involve many more number of Relay ops, which might degrade performance. So, to enable TVM user to choose different accuracy-performance tradeoff points, we can add a new argument in the TFLite parser API, and then pass it along to the QNN ops. The default value of this arg can be tflite rounding. @FrozenGene Are you ok with this? Yes, this is my thought too. I am ok with it. > LGTM. I like the scope of this PR. IMO, we can get this in. And send other PRs for TFLite parser. IMO, I would prefer adding TFLite parser in this PR too. Because this will be organized into one complete solution for TFLite rounding. When others review the history, they don't need to check isolated PRs together to get complete support for TFLite rounding support. Maybe these PRs during time is long. It is right that we could make small prs and merge it in quickly so that we could get functionality supporting quickly and could locate the reason quickly. However, from this pr's perspective, TFLite's parser code will not be complex and could be contained in. It is acceptable in one single PR from my opinion. So I would suggest @LiangHao151941 adding TFLite parser in this PR too. If we think it is import, we even could port it back to release 0.6, one PR will make us do it easily too. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
