-
Notifications
You must be signed in to change notification settings - Fork 48
add: detect Q/DQ with int16/uint16 initializers for GPU Scale Transform Pass #768
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
base: ovep-develop
Are you sure you want to change the base?
Conversation
@mklimenk Please test, review & merge |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add tests to the IsQDQGraphWithUint16OrInt16()
function to make sure that we cover all the cases.
Also please remove excessive comments, there's no need to be that explicit.
auto is_16bit_tensor = [](const onnxruntime::NodeArg* node_arg) -> bool { | ||
if (!node_arg) return false; | ||
const auto* type_proto = node_arg->TypeAsProto(); | ||
if (type_proto && type_proto->has_tensor_type()) { | ||
auto elem_type = type_proto->tensor_type().elem_type(); | ||
return (elem_type == ONNX_NAMESPACE::TensorProto_DataType_UINT16 || | ||
elem_type == ONNX_NAMESPACE::TensorProto_DataType_INT16); | ||
} | ||
return false; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please move it to a separate function, there's no need for long multi-line lambdas
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
// QuantizeLinear: [float_input, scale, zero_point] -> [quantized_output] | ||
// The quantized output tensor determines the quantization type |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove identical comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
|
||
// Zero point (index 2) must match quantized tensor type per ONNX spec | ||
// It's optional - absent for INT32 and some float8 types | ||
if (input_defs.size() >= 3 && is_16bit_tensor(input_defs[2])) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it be output_defs[2]
? It seems like the portion in the previous condition
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes its the zero_point dtype that is tested to check the INT16/UINT16 dtype
fbf966a
to
6ceb8e7
Compare
Description
This PR enables the GPU Scale Transform Pass by detecting the UINT16 & INT16 Initializers type in the Q/DQ Nodes in the graph. This isolates the dependency on using the enable_qdq_optimizer provider option pass in the legacy OVEP code.