[quant][pt2e] Refactor insert observer to do sharing checking in the same place #113458

jerryzh168 · 2023-11-10T17:25:10Z

Stack from ghstack (oldest at bottom):

[quant][pt2e] Refactor some internal code for observer insertion #113500
-> [quant][pt2e] Refactor insert observer to do sharing checking in the same place #113458

Summary:
Previously it is scatter in two different places: before inserting observer and during observer,
this PR moved everything before we insert observer

Next: refactor QuantizationSpec and check more fields for sharing

Test Plan:
CI (regression tests)

Reviewers:

Subscribers:

Tasks:

Tags:

…same place Summary: Previously it is scatter in two different places: before inserting observer and during observer, this PR moved everything before we insert observer * Next: refactor QuantizationSpec and check more fields for sharing Test Plan: CI (regression tests) Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

pytorch-bot · 2023-11-10T17:25:15Z

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/113458

📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓ Need help or want to give feedback on the CI? Visit the bot commands wiki or our office hours

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit 23e2da8 with merge base 2a271a3 ():
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

…same place Summary: Previously it is scatter in two different places: before inserting observer and during observer, this PR moved everything before we insert observer * Next: refactor QuantizationSpec and check more fields for sharing Test Plan: CI (regression tests) Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: d515759 Pull Request resolved: #113458

…ing in the same place" Summary: Previously it is scatter in two different places: before inserting observer and during observer, this PR moved everything before we insert observer * Next: refactor QuantizationSpec and check more fields for sharing Test Plan: CI (regression tests) Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

kimishpatel · 2023-11-15T15:35:51Z

torch/ao/quantization/pt2e/prepare.py

+
+                # sharing with other users of the previous output
+                # (arg, user)
+                for user in arg.users:


This is not a refactor change, right?

this is a refactor, we removed this logic from insert observers and added it here

kimishpatel

left a comment

jerryzh168 · 2023-11-15T18:26:20Z

@pytorchbot merge

pytorchmergebot · 2023-11-15T18:28:19Z

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

huydhn · 2023-11-16T20:41:34Z

@pytorchbot revert -m 'Sorry for reverting your change but it is failing executorch export test for llama2' -c ghfirst

The error is:

======================================================================
ERROR: test_llama_qnn (executorch.examples.models.llama2.fb.test_llama_qnn.LlamaLiteModelExportTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/data/sandcastle/boxes/eden-trunk-hg-fbcode-fbsource/buck-out/v2/gen/fbcode/24c04fc3eb9063ab/executorch/examples/models/llama2/fb/__test_llama_qnn__/test_llama_qnn#link-tree/executorch/examples/models/llama2/fb/test_llama_qnn.py", line 70, in test_llama_qnn
    prepared_gm = prepare_pt2e(captured_gm, QnnHtpQuantizer())
  File "/data/sandcastle/boxes/eden-trunk-hg-fbcode-fbsource/buck-out/v2/gen/fbcode/24c04fc3eb9063ab/executorch/examples/models/llama2/fb/__test_llama_qnn__/test_llama_qnn#link-tree/torch/ao/quantization/quantize_pt2e.py", line 109, in prepare_pt2e
    model = prepare(model, node_name_to_scope, is_qat=False)
  File "/data/sandcastle/boxes/eden-trunk-hg-fbcode-fbsource/buck-out/v2/gen/fbcode/24c04fc3eb9063ab/executorch/examples/models/llama2/fb/__test_llama_qnn__/test_llama_qnn#link-tree/torch/ao/quantization/pt2e/prepare.py", line 442, in prepare
    edge_or_node_to_group_id = _get_edge_or_node_to_group_id(edge_or_node_to_qspec)
  File "/data/sandcastle/boxes/eden-trunk-hg-fbcode-fbsource/buck-out/v2/gen/fbcode/24c04fc3eb9063ab/executorch/examples/models/llama2/fb/__test_llama_qnn__/test_llama_qnn#link-tree/torch/ao/quantization/pt2e/prepare.py", line 206, in _get_edge_or_node_to_group_id
    for user in arg.users:
AttributeError: 'int' object has no attribute 'users'

----------------------------------------------------------------------
Ran 1 test in 6.339s

Just FYI, I'm working on bring this test to OSS https://github.com/pytorch/pytorch/blob/main/.ci/pytorch/test.sh#L1014-L1016

pytorchmergebot · 2023-11-16T20:43:34Z

@pytorchbot successfully started a revert job. Check the current status here.
Questions? Feedback? Please reach out to the PyTorch DevX Team

pytorchmergebot · 2023-11-16T20:43:45Z

@jerryzh168 your PR has been successfully reverted.

… in the same place (#113458)" This reverts commit 585e315. Reverted #113458 on behalf of https://github.com/huydhn due to Sorry for reverting your change but it is failing executorch export test for llama2 ([comment](#113458 (comment)))

… in the same place (#113458) Summary: Previously it is scatter in two different places: before inserting observer and during observer, this PR moved everything before we insert observer * Next: refactor QuantizationSpec and check more fields for sharing Test Plan: CI (regression tests) Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

… in the same place (#113458) Summary: Previously it is scatter in two different places: before inserting observer and during observer, this PR moved everything before we insert observer * Next: refactor QuantizationSpec and check more fields for sharing Test Plan: CI (regression tests) Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: dadc425 Pull Request resolved: #113920

jerryzh168 · 2023-11-17T04:56:27Z

@pytorchbot revert -m 'Sorry for reverting your change but it is failing executorch export test for llama2' -c ghfirst

The error is:

======================================================================
ERROR: test_llama_qnn (executorch.examples.models.llama2.fb.test_llama_qnn.LlamaLiteModelExportTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/data/sandcastle/boxes/eden-trunk-hg-fbcode-fbsource/buck-out/v2/gen/fbcode/24c04fc3eb9063ab/executorch/examples/models/llama2/fb/__test_llama_qnn__/test_llama_qnn#link-tree/executorch/examples/models/llama2/fb/test_llama_qnn.py", line 70, in test_llama_qnn
    prepared_gm = prepare_pt2e(captured_gm, QnnHtpQuantizer())
  File "/data/sandcastle/boxes/eden-trunk-hg-fbcode-fbsource/buck-out/v2/gen/fbcode/24c04fc3eb9063ab/executorch/examples/models/llama2/fb/__test_llama_qnn__/test_llama_qnn#link-tree/torch/ao/quantization/quantize_pt2e.py", line 109, in prepare_pt2e
    model = prepare(model, node_name_to_scope, is_qat=False)
  File "/data/sandcastle/boxes/eden-trunk-hg-fbcode-fbsource/buck-out/v2/gen/fbcode/24c04fc3eb9063ab/executorch/examples/models/llama2/fb/__test_llama_qnn__/test_llama_qnn#link-tree/torch/ao/quantization/pt2e/prepare.py", line 442, in prepare
    edge_or_node_to_group_id = _get_edge_or_node_to_group_id(edge_or_node_to_qspec)
  File "/data/sandcastle/boxes/eden-trunk-hg-fbcode-fbsource/buck-out/v2/gen/fbcode/24c04fc3eb9063ab/executorch/examples/models/llama2/fb/__test_llama_qnn__/test_llama_qnn#link-tree/torch/ao/quantization/pt2e/prepare.py", line 206, in _get_edge_or_node_to_group_id
    for user in arg.users:
AttributeError: 'int' object has no attribute 'users'

----------------------------------------------------------------------
Ran 1 test in 6.339s

Just FYI, I'm working on bring this test to OSS https://github.com/pytorch/pytorch/blob/main/.ci/pytorch/test.sh#L1014-L1016

this feels like an error on HtpQnnQuantizer side actually, I'll check with Shen

…rver to do sharing checking in the same place (#113458)" Summary: Previously it is scatter in two different places: before inserting observer and during observer, this PR moved everything before we insert observer * Next: refactor QuantizationSpec and check more fields for sharing Test Plan: CI (regression tests) Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D51420029](https://our.internmc.facebook.com/intern/diff/D51420029) [ghstack-poisoned]

… in the same place (#113458) Summary: Previously it is scatter in two different places: before inserting observer and during observer, this PR moved everything before we insert observer * Next: refactor QuantizationSpec and check more fields for sharing Test Plan: CI (regression tests) Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 9888739 Pull Request resolved: #113920

…ng checking in the same place (#113458)" Summary: Previously it is scatter in two different places: before inserting observer and during observer, this PR moved everything before we insert observer * Next: refactor QuantizationSpec and check more fields for sharing Test Plan: CI (regression tests) Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D51420029](https://our.internmc.facebook.com/intern/diff/D51420029) [ghstack-poisoned]

jerryzh168 · 2023-11-21T23:37:44Z

reland PR: #113920

… in the same place (#113458) (#113920) Summary: Previously it is scatter in two different places: before inserting observer and during observer, this PR moved everything before we insert observer * Next: refactor QuantizationSpec and check more fields for sharing Test Plan: CI (regression tests) Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D51420029](https://our.internmc.facebook.com/intern/diff/D51420029) Pull Request resolved: #113920 Approved by: https://github.com/andrewor14

pytorch-bot bot added the release notes: AO frontend label Nov 10, 2023

github-actions bot added the release notes: quantization release notes category label Nov 10, 2023

jerryzh168 requested review from andrewor14 and kimishpatel November 11, 2023 00:20

jerryzh168 mentioned this pull request Nov 11, 2023

[quant][pt2e] Refactor some internal code for observer insertion #113500

Closed

jerryzh168 added the topic: not user facing topic category label Nov 11, 2023

kimishpatel reviewed Nov 15, 2023

View reviewed changes

kimishpatel approved these changes Nov 15, 2023

View reviewed changes

pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Nov 15, 2023

pytorchmergebot added the merging label Nov 15, 2023

pytorchmergebot added the Merged label Nov 15, 2023

pytorchmergebot closed this in 585e315 Nov 15, 2023

pytorchmergebot removed the merging label Nov 15, 2023

pytorchmergebot added the Reverted label Nov 16, 2023

pytorchmergebot reopened this Nov 16, 2023

jerryzh168 closed this Nov 21, 2023

facebook-github-bot deleted the gh/jerryzh168/933/head branch November 25, 2023 15:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[quant][pt2e] Refactor insert observer to do sharing checking in the same place #113458

[quant][pt2e] Refactor insert observer to do sharing checking in the same place #113458

Uh oh!

jerryzh168 commented Nov 10, 2023 •

edited

Loading

Uh oh!

pytorch-bot bot commented Nov 10, 2023 •

edited

Loading

Uh oh!

kimishpatel Nov 15, 2023

Uh oh!

jerryzh168 Nov 15, 2023 •

edited

Loading

Uh oh!

kimishpatel left a comment

Uh oh!

jerryzh168 commented Nov 15, 2023

Uh oh!

pytorchmergebot commented Nov 15, 2023

Uh oh!

huydhn commented Nov 16, 2023

Uh oh!

pytorchmergebot commented Nov 16, 2023

Uh oh!

pytorchmergebot commented Nov 16, 2023

Uh oh!

jerryzh168 commented Nov 17, 2023

Uh oh!

jerryzh168 commented Nov 21, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

[quant][pt2e] Refactor insert observer to do sharing checking in the same place #113458

[quant][pt2e] Refactor insert observer to do sharing checking in the same place #113458

Uh oh!

Conversation

jerryzh168 commented Nov 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Nov 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/113458

✅ No Failures

Uh oh!

kimishpatel Nov 15, 2023

Choose a reason for hiding this comment

Uh oh!

jerryzh168 Nov 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kimishpatel left a comment

Choose a reason for hiding this comment

Uh oh!

jerryzh168 commented Nov 15, 2023

Uh oh!

pytorchmergebot commented Nov 15, 2023

Merge started

Uh oh!

huydhn commented Nov 16, 2023

Uh oh!

pytorchmergebot commented Nov 16, 2023

Uh oh!

pytorchmergebot commented Nov 16, 2023

Uh oh!

jerryzh168 commented Nov 17, 2023

Uh oh!

jerryzh168 commented Nov 21, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jerryzh168 commented Nov 10, 2023 •

edited

Loading

pytorch-bot bot commented Nov 10, 2023 •

edited

Loading

jerryzh168 Nov 15, 2023 •

edited

Loading