-
Notifications
You must be signed in to change notification settings - Fork 89
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
Add QLinearMul operator #2430
Add QLinearMul operator #2430
Conversation
This should reuse the struct parse_qlinearbinary : op_parser<parse_qlinearbinary>
{
std::vector<op_desc> operators() const { return {{"QLinearAdd", "add"}, {"QLinearMul", "mul"}}; }
void check_inputs(...) const
{
...
}
instruction_ref parse(const op_desc& opd,
const onnx_parser& /*parser*/,
const onnx_parser::node_info& info,
const std::vector<instruction_ref>& args) const
{
check_inputs(args);
// A
const auto& in_a = args[0];
const auto& in_scale_a = args[1];
const auto& in_zero_pt_a = args[2];
auto dquant_a = bcast_qdq_instr("dequantizelinear", in_a, in_scale_a, in_zero_pt_a, info);
// B
const auto& in_b = args[3];
const auto& in_scale_b = args[4];
const auto& in_zero_pt_b = args[5];
auto dquant_b = bcast_qdq_instr("dequantizelinear", in_b, in_scale_b, in_zero_pt_b, info);
// C = A + B
auto out_c = info.add_common_op(opd.op_name, dquant_a, dquant_b);
const auto& in_scale_c = args[6];
// zero_pt for C is supplied as the last optional argument..
if(args.size() == 8)
return (bcast_qdq_instr("quantizelinear", out_c, in_scale_c, args[7], info));
// if no zero_pt: just broadcast the scale..
auto bcast_scale_c = bcast_scalar_instr(out_c->get_shape(), in_scale_c, info);
return (info.add_instruction(migraphx::make_op("quantizelinear"), out_c, bcast_scale_c));
}
}; |
This build is not recommended to merge 🔴 |
❌agentmodel: ERROR - check error outputTraceback (most recent call last):File "/src/AMDMIGraphX/tools/accuracy/accuracy_checker.py", line 336, in main() File "/src/AMDMIGraphX/tools/accuracy/accuracy_checker.py", line 254, in main pred_migx = np.array(model.run(params)[-1]) RuntimeError: /src/AMDMIGraphX/src/targets/gpu/device/include/migraphx/gpu/device/visit.hpp:140: hip_visit_views_impl: Ranks must be the same 🔴bert_base_cased_fp16: FAILED: MIGraphX is not within tolerance - check verbose output🔴bert_large_uncased_fp16: FAILED: MIGraphX is not within tolerance - check verbose output🔴distilgpt2_fp16: FAILED: MIGraphX is not within tolerance - check verbose output |
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.
Could you please help me understand how you could use the exact test data-set from qlinearadd** tests to verify for qlinearmul_test & qlinear_bcast_test. The compute results for Add and Mul operations should be (very) different. Thanks.
@lakhinderwalia I've lowered the scales for A and B from |
26e8dc7
to
9a5db27
Compare
@pfultz2 @lakhinderwalia I've updated the PR with the proposed |
2fe89ff
to
846ccc3
Compare
Please note, all the output values that you are testing/verifying are zeros. This test-vector originally designed for Add isn't right for Multiplication. It is likely that your scale is so small that it is all effectively converting to zeros -- and the test passes. Thanks. |
846ccc3
to
4534fcd
Compare
@lakhinderwalia I've changed the scale values so the in between values are not zeros, also updated the results according to that. |
test/onnx/gen_onnx.py
Outdated
zero_pt_a = helper.make_tensor('A_zero_point', TensorProto.UINT8, [], [0]) | ||
|
||
b = helper.make_tensor_value_info('B', TensorProto.UINT8, [64]) | ||
sc_b = helper.make_tensor('B_scale', TensorProto.FLOAT, [], [-0.05]) |
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.
Quantization scale is always a positive number.
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.
Changed it to positive number, and the test results according to that.
4534fcd
to
566907f
Compare
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.
Thank you.
QLinearMul operator is required for the DenseNet-int8 onnx zoo model.
Fixes: migraphx-benchmark#150