Fix the duplicated QDQ attributes setup issue #18039

vera121 · 2023-10-20T10:19:38Z

Description

The copied QDQ node should have exactly the same attributes as the original QDQ node. Otherwise, it might cause errors when the original node has attributes that use non default values (such as axis != 1 case).

An example user case is like:
A DequantizeLinear node has more than 1 consumer in the graph, and its attributes axis is 0.

Motivation and Context

I see the errors like
#16188
and this fix could solve the issue.

The copied QDQ node should have exactly the same attributes as the original QDQ node. Otherwise, it might cause errors when the original node has attributes that use non default values (such as axis != 1 case).

edgchen1 · 2023-10-20T16:55:46Z

thanks for the fix. could you please also add a unit test for it?

edgchen1

thanks for adding the test. just had one comment about it.

onnxruntime/test/optimizer/ensure_unique_dq_for_node_unit_test.cc

edgchen1 · 2023-10-24T17:17:14Z

/azp run Linux CPU CI Pipeline

azure-pipelines · 2023-10-24T17:17:26Z

Azure Pipelines successfully started running 1 pipeline(s).

edgchen1 · 2023-10-25T21:47:15Z

/azp run Linux CPU CI Pipeline, Linux CPU Minimal Build E2E CI Pipeline, Linux GPU CI Pipeline, Linux GPU TensorRT CI Pipeline, Linux OpenVINO CI Pipeline, MacOS CI Pipeline, ONNX Runtime Web CI Pipeline, onnxruntime-binary-size-checks-ci-pipeline, Linux QNN CI Pipeline

edgchen1 · 2023-10-25T21:47:40Z

/azp run Windows CPU CI Pipeline, Windows GPU CI Pipeline, Windows GPU TensorRT CI Pipeline, Windows ARM64 QNN CI Pipeline, orttraining-linux-ci-pipeline, orttraining-linux-gpu-ci-pipeline, orttraining-ortmodule-distributed, ONNX Runtime React Native CI Pipeline, Windows x64 QNN CI Pipeline

azure-pipelines · 2023-10-25T21:47:56Z

Azure Pipelines successfully started running 9 pipeline(s).

edgchen1 · 2023-10-25T21:48:07Z

/azp run Linux MIGraphX CI Pipeline, orttraining-amd-gpu-ci-pipeline

azure-pipelines · 2023-10-25T21:48:16Z

Azure Pipelines successfully started running 9 pipeline(s).

azure-pipelines · 2023-10-25T21:48:20Z

Azure Pipelines successfully started running 2 pipeline(s).

edgchen1 · 2023-10-26T03:44:13Z

the PR looks pretty good to me.

the formatting check is complaining:
https://github.com/microsoft/onnxruntime/actions/runs/6634815370/job/18060070638?pr=18039

please run clang-format on the modified files. this is one way to set that up:
https://github.com/microsoft/onnxruntime/blob/main/docs/Coding_Conventions_and_Standards.md#linting

edgchen1 · 2023-10-26T16:55:26Z

/azp run Linux CPU CI Pipeline, Linux CPU Minimal Build E2E CI Pipeline, Linux GPU CI Pipeline, Linux GPU TensorRT CI Pipeline, Linux OpenVINO CI Pipeline, MacOS CI Pipeline, ONNX Runtime Web CI Pipeline, onnxruntime-binary-size-checks-ci-pipeline, Linux QNN CI Pipeline

edgchen1 · 2023-10-26T16:55:35Z

/azp run Windows CPU CI Pipeline, Windows GPU CI Pipeline, Windows GPU TensorRT CI Pipeline, Windows ARM64 QNN CI Pipeline, orttraining-linux-ci-pipeline, orttraining-linux-gpu-ci-pipeline, orttraining-ortmodule-distributed, ONNX Runtime React Native CI Pipeline, Windows x64 QNN CI Pipeline

edgchen1 · 2023-10-26T16:55:45Z

/azp run Linux MIGraphX CI Pipeline, orttraining-amd-gpu-ci-pipeline

azure-pipelines · 2023-10-26T16:55:57Z

Azure Pipelines successfully started running 2 pipeline(s).

azure-pipelines · 2023-10-26T16:56:17Z

Azure Pipelines successfully started running 9 pipeline(s).

azure-pipelines · 2023-10-26T16:58:43Z

Azure Pipelines successfully started running 9 pipeline(s).

edgchen1 · 2023-10-30T19:32:26Z

@vera121 I think this PR is almost ready to merge. would you mind taking a look at the Contributor License Agreement?

#18039 (comment)

vera121 · 2023-10-31T03:42:58Z

@vera121 I think this PR is almost ready to merge. would you mind taking a look at the Contributor License Agreement?

#18039 (comment)

Hello @edgchen1
Thanks for the remind. I am consulting my company's legal department to get their approval for this assigment. After that I will reply to that message and notify you as well.

edgchen1 · 2023-12-01T23:33:02Z

@vera121 I think this PR is almost ready to merge. would you mind taking a look at the Contributor License Agreement?
#18039 (comment)

Hello @edgchen1 Thanks for the remind. I am consulting my company's legal department to get their approval for this assigment. After that I will reply to that message and notify you as well.

Hi @vera121, just wanted to follow up on this PR. were you able to get the necessary approvals?

vera121 · 2023-12-04T03:30:20Z

@vera121 I think this PR is almost ready to merge. would you mind taking a look at the Contributor License Agreement?
#18039 (comment)

Hello @edgchen1 Thanks for the remind. I am consulting my company's legal department to get their approval for this assigment. After that I will reply to that message and notify you as well.

Hi @vera121, just wanted to follow up on this PR. were you able to get the necessary approvals?

Hello @edgchen1 Thanks for the remind. Yes, I didn't forget this but our company's legal department is still running the reviewing process (you know such process usually might take a while). I will notify you and submit the CLA as soon as they finish. Thanks very much.

vera121 · 2024-01-11T02:15:05Z

@vera121 please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
@microsoft-github-policy-service agree [company="{your company}"]
Options:

(default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@microsoft-github-policy-service agree
(when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@microsoft-github-policy-service agree company="Microsoft"
Contributor License Agreement

@microsoft-github-policy-service agree [company="Synopsys"]

vera121 · 2024-01-11T02:17:34Z

@vera121 I think this PR is almost ready to merge. would you mind taking a look at the Contributor License Agreement?
#18039 (comment)

Hello @edgchen1 Thanks for the remind. I am consulting my company's legal department to get their approval for this assigment. After that I will reply to that message and notify you as well.

Hi @vera121, just wanted to follow up on this PR. were you able to get the necessary approvals?

Hello @edgchen1 Sorry for the long wait. Now we finally got the approval from our company's legal department. Please check my reply and go on with the merge.
Thanks very much for the help!

vera121 · 2024-01-11T02:18:59Z

@vera121 please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
@microsoft-github-policy-service agree [company="{your company}"]
Options:

(default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@microsoft-github-policy-service agree
(when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@microsoft-github-policy-service agree company="Microsoft"
Contributor License Agreement

@microsoft-github-policy-service agree company="Synopsys"

edgchen1 · 2024-01-11T02:37:06Z

great, just in time for the 1.17 release! thanks again for the fix!

Fix the duplicated QDQ attributes setup issue

0328a82

The copied QDQ node should have exactly the same attributes as the original QDQ node. Otherwise, it might cause errors when the original node has attributes that use non default values (such as axis != 1 case).

vera121 added 2 commits October 23, 2023 12:56

upload simple test model for qdq muti consumer case with axis!=1

919a486

add unit test for qdq model with multi consumer and axis!=1 attr

bd51001

edgchen1 reviewed Oct 23, 2023

View reviewed changes

onnxruntime/test/optimizer/ensure_unique_dq_for_node_unit_test.cc Show resolved Hide resolved

update the unit test to check attr axis=0 nodes explicitly

5186208

edgchen1 reviewed Oct 24, 2023

View reviewed changes

refine coding styles according to suggestion

da68ad1

refine coding styles to solve lint warnings

7424d1f

edgchen1 approved these changes Oct 26, 2023

View reviewed changes

edgchen1 mentioned this pull request Dec 2, 2023

Handle non-default DequantizeLinear axis in DuplicateDQForOutputEdge #18669

Closed

edgchen1 merged commit 5678317 into microsoft:main Jan 11, 2024
68 of 70 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix the duplicated QDQ attributes setup issue #18039

Fix the duplicated QDQ attributes setup issue #18039

vera121 commented Oct 20, 2023

edgchen1 commented Oct 20, 2023

edgchen1 left a comment

edgchen1 commented Oct 24, 2023

azure-pipelines bot commented Oct 24, 2023

edgchen1 commented Oct 25, 2023

edgchen1 commented Oct 25, 2023

azure-pipelines bot commented Oct 25, 2023

edgchen1 commented Oct 25, 2023

azure-pipelines bot commented Oct 25, 2023

azure-pipelines bot commented Oct 25, 2023

edgchen1 commented Oct 26, 2023

edgchen1 commented Oct 26, 2023

edgchen1 commented Oct 26, 2023

edgchen1 commented Oct 26, 2023

azure-pipelines bot commented Oct 26, 2023

azure-pipelines bot commented Oct 26, 2023

azure-pipelines bot commented Oct 26, 2023

edgchen1 commented Oct 30, 2023

vera121 commented Oct 31, 2023

edgchen1 commented Dec 1, 2023

vera121 commented Dec 4, 2023

vera121 commented Jan 11, 2024

vera121 commented Jan 11, 2024

vera121 commented Jan 11, 2024

edgchen1 commented Jan 11, 2024

Fix the duplicated QDQ attributes setup issue #18039

Fix the duplicated QDQ attributes setup issue #18039

Conversation

vera121 commented Oct 20, 2023

Description

Motivation and Context

edgchen1 commented Oct 20, 2023

edgchen1 left a comment

Choose a reason for hiding this comment

edgchen1 commented Oct 24, 2023

azure-pipelines bot commented Oct 24, 2023

edgchen1 commented Oct 25, 2023

edgchen1 commented Oct 25, 2023

azure-pipelines bot commented Oct 25, 2023

edgchen1 commented Oct 25, 2023

azure-pipelines bot commented Oct 25, 2023

azure-pipelines bot commented Oct 25, 2023

edgchen1 commented Oct 26, 2023

edgchen1 commented Oct 26, 2023

edgchen1 commented Oct 26, 2023

edgchen1 commented Oct 26, 2023

azure-pipelines bot commented Oct 26, 2023

azure-pipelines bot commented Oct 26, 2023

azure-pipelines bot commented Oct 26, 2023

edgchen1 commented Oct 30, 2023

vera121 commented Oct 31, 2023

edgchen1 commented Dec 1, 2023

vera121 commented Dec 4, 2023

vera121 commented Jan 11, 2024

vera121 commented Jan 11, 2024

vera121 commented Jan 11, 2024

edgchen1 commented Jan 11, 2024