microsoft / onnxruntime

ONNX Runtime: cross-platform, high performance ML inferencing and training accelerator
https://onnxruntime.ai
MIT License
13.66k stars 2.78k forks source link

When will onnxruntime make it available to get nodeunit inputs correctly for QLinearConcat #20830

Open chenfeiyue-cfy opened 1 month ago

chenfeiyue-cfy commented 1 month ago

Describe the issue

May I ask when will onnxruntime support for setting inputs for the QLinearConcat operator in node unit structure be available? I saw a ‘todo’ note in the code, but there has been no progress for the past two years.” 企业微信截图_1716798582784

To reproduce

File path:node_unit.cc. I found that onnxruntime does not handle the inputs of the QLinearConcat's node_unit in the same way as it does with other QLinear operators, that is, by binding each actual input with its quantization information into a single input.

Urgency

No response

Platform

Linux

OS Version

Ubuntu 22.04.2 LTS

ONNX Runtime Installation

Built from Source

ONNX Runtime Version or Commit ID

main

ONNX Runtime API

C++

Architecture

X86

Execution Provider

Default CPU, NNAPI, Other / Unknown

Execution Provider Library Version

No response

skottmckay commented 1 month ago

Do you have a model where this causes an issue?

chenfeiyue-cfy commented 1 month ago

Sure, the model is downloaded from onnx model zoo:(https://github.com/onnx/models/blob/main/validated/vision/classification/densenet-121/model/densenet-12-int8.onnx). The unit test is from path onnxruntime/onnxruntime/test/contrib_ops/qlinear_concat_test.cc(https://github.com/microsoft/onnxruntime/blob/main/onnxruntime/test/contrib_ops/qlinear_concat_test.cc)

skottmckay commented 1 month ago

I don't think the lack of support makes a difference in this case.

QLinearConcat is an internal operator, so we would expect it to be handled by our internal execution providers. In that scenario, the fact that the zp/scale values aren't nicely provided by the NodeUnit doesn't change anything as nothing (AFAIK) is trying to read the zp and scale from the NodeUnit as we have static kernels for CPU/CUDA/DML execution providers to handle QLinearConcat.

Really for the model to be in the ONNX repo it should be in QDQ format (DequantizeLinear on each input with a Concat node and a QuantizeLinear on the output) instead of having been saved using internal onnxruntime operators. With that model format any execution provider could convert the set of nodes into a quantized concat operation, and it would want the zp/scale for the inputs to be conveniently available via the NodeUnit. That's a different NodeUnit constructor though and not the path with the TODO comment.

chenfeiyue-cfy commented 1 month ago

Sure it is not an error occured, but in the node_unit.cc file, I saw QLinearMatMul and QLinearConv opshave been handled like that .QDQMatmul/QDQConv have made node_arg and quant_param binded together with one single input for node_unitI made this request because our execution provider treats quantization operations and normal operations equally.If QLinearConcat could be easier to get input like QLinearConv it will make our provider work more efficiently.

skottmckay commented 1 month ago

I have added an item to the backlog to look at adding the quantization info to a NodeUnit for the QLinearX operators. No guarantee when we'll get to it though.