[ao][ns] Replacing List[QConfigMapping] in PNP #86922

HDCharles · 2022-10-13T18:00:58Z

Stack from ghstack (oldest at bottom):

-> [ao][ns] Replacing List[QConfigMapping] in PNP #86922

Summary: Added QConfigMultiMapping which is essentially a
List[QConfigMapping] with set methods and dedicated handling to
avoid unwanted matches and improve UX.

note: the from future import annotations line caused weird errors when the
QConfigMultiMapping class was put in _numeric_suite_fx.py so it was moved.

Test Plan: python test/test_quantization.py TestFxNumericSuiteNShadows

Reviewers:

Subscribers:

Tasks:

Tags:

Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

pytorch-bot · 2022-10-13T18:01:03Z

🔗 Helpful Links

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

📄 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 our office hours

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

✅ No Failures

As of commit 53951ba:
💚 Looks good so far! There are no failures yet. 💚

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

Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 93cf9ae0015176e092d6589ec04d4526131213ad Pull Request resolved: #86922

Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 3896ee91b73961b32f7c3d103717436ba8ae9b4f Pull Request resolved: #86922

Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 92353bd57a26786fdec554aba5233c2273ac2e16 Pull Request resolved: #86922

Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

Summary: Added QConfigMultiMapping which is essentially a List[QConfigMapping] with set methods and dedicated handling to avoid unwanted matches and improve UX. Test Plan: python test/test_quantization.py TestFxNumericSuiteNShadows Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 20b6e24479797211232ecc3ae53a67c179348b7f Pull Request resolved: #86922

Summary: Added QConfigMultiMapping which is essentially a List[QConfigMapping] with set methods and dedicated handling to avoid unwanted matches and improve UX. Test Plan: python test/test_quantization.py TestFxNumericSuiteNShadows Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

Summary: Added QConfigMultiMapping which is essentially a List[QConfigMapping] with set methods and dedicated handling to avoid unwanted matches and improve UX. Test Plan: python test/test_quantization.py TestFxNumericSuiteNShadows Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 20b6e24479797211232ecc3ae53a67c179348b7f Pull Request resolved: #86922

Summary: Added QConfigMultiMapping which is essentially a List[QConfigMapping] with set methods and dedicated handling to avoid unwanted matches and improve UX. Test Plan: python test/test_quantization.py TestFxNumericSuiteNShadows Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

Summary: Added QConfigMultiMapping which is essentially a List[QConfigMapping] with set methods and dedicated handling to avoid unwanted matches and improve UX. Test Plan: python test/test_quantization.py TestFxNumericSuiteNShadows Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 072aaabd693cafe6309ef139d7acd8f944c8068c Pull Request resolved: #86922

Summary: Added QConfigMultiMapping which is essentially a List[QConfigMapping] with set methods and dedicated handling to avoid unwanted matches and improve UX. Test Plan: python test/test_quantization.py TestFxNumericSuiteNShadows Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

Summary: Added QConfigMultiMapping which is essentially a List[QConfigMapping] with set methods and dedicated handling to avoid unwanted matches and improve UX. Test Plan: python test/test_quantization.py TestFxNumericSuiteNShadows Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: fdaa02f8016521acdb0941c63efa6b4c9caf026d Pull Request resolved: #86922

Summary: Added QConfigMultiMapping which is essentially a List[QConfigMapping] with set methods and dedicated handling to avoid unwanted matches and improve UX. Test Plan: python test/test_quantization.py TestFxNumericSuiteNShadows Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

Summary: Added QConfigMultiMapping which is essentially a List[QConfigMapping] with set methods and dedicated handling to avoid unwanted matches and improve UX. Test Plan: python test/test_quantization.py TestFxNumericSuiteNShadows Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 532dae4890de9ab3b35431ed3aa1abdeb9760f38 Pull Request resolved: #86922

Summary: Added QConfigMultiMapping which is essentially a List[QConfigMapping] with set methods and dedicated handling to avoid unwanted matches and improve UX. note: the from __future__ import annotations line caused weird errors when the QConfigMultiMapping class was put in _numeric_suite_fx.py so it was moved. Test Plan: python test/test_quantization.py TestFxNumericSuiteNShadows Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

Summary: Added QConfigMultiMapping which is essentially a List[QConfigMapping] with set methods and dedicated handling to avoid unwanted matches and improve UX. note: the from __future__ import annotations line caused weird errors when the QConfigMultiMapping class was put in _numeric_suite_fx.py so it was moved. Test Plan: python test/test_quantization.py TestFxNumericSuiteNShadows Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: b0750baff8f69423b86a6272d7565b890ea9c634 Pull Request resolved: #86922

Summary: Added QConfigMultiMapping which is essentially a List[QConfigMapping] with set methods and dedicated handling to avoid unwanted matches and improve UX. note: the from __future__ import annotations line caused weird errors when the QConfigMultiMapping class was put in _numeric_suite_fx.py so it was moved. Test Plan: python test/test_quantization.py TestFxNumericSuiteNShadows Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

Summary: Added QConfigMultiMapping which is essentially a List[QConfigMapping] with set methods and dedicated handling to avoid unwanted matches and improve UX. note: the from __future__ import annotations line caused weird errors when the QConfigMultiMapping class was put in _numeric_suite_fx.py so it was moved. Test Plan: python test/test_quantization.py TestFxNumericSuiteNShadows Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 4dc008b037383105ec43e46eb0fea70304191002 Pull Request resolved: #86922

Summary: Added QConfigMultiMapping which is essentially a List[QConfigMapping] with set methods and dedicated handling to avoid unwanted matches and improve UX. note: the from __future__ import annotations line caused weird errors when the QConfigMultiMapping class was put in _numeric_suite_fx.py so it was moved. Test Plan: python test/test_quantization.py TestFxNumericSuiteNShadows Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

Summary: Added QConfigMultiMapping which is essentially a List[QConfigMapping] with set methods and dedicated handling to avoid unwanted matches and improve UX. note: the from __future__ import annotations line caused weird errors when the QConfigMultiMapping class was put in _numeric_suite_fx.py so it was moved. Test Plan: python test/test_quantization.py TestFxNumericSuiteNShadows Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 6fd6a774a3c058c2e8363d3ee386248e3726d151 Pull Request resolved: #86922

vkuzo · 2022-10-24T17:49:01Z

torch/ao/ns/_numeric_suite_fx.py

@@ -753,7 +754,7 @@ def extend_logger_results_with_comparison(
 def prepare_n_shadows_model(
    model: torch.nn.Module,
    example_inputs: Any,
-    qconfig_mappings: List[QConfigMapping],
+    qconfig_mappings: Union[List[QConfigMapping], QConfigMultiMapping],


can we just take the new object instead of old object or new object?

I can do that, just didn't want to break BC unnecessarily when its easy enough to maintain

No one is using it yet, so there is no expectation of BC. We can change API as needed until we get some customers, in fact this is the time when it's easiest to change the API.

vkuzo · 2022-10-24T17:51:04Z

torch/ao/ns/fx/qconfig_multi_mapping.py

+            while len(qconfig_list) < len(self.qconfig_mappings_list):
+                qconfig_list.append(None)
+
+    def _remove_duplicates(self, qconfig_list) -> None:


maybe move this outside the class since it's not changing any class variables?

vkuzo · 2022-10-24T19:22:07Z

torch/ao/ns/fx/qconfig_multi_mapping.py

+}
+
+
+class QConfigMultiMapping:


any thoughts on how the user should think about the order of the results? For example, if the user does

QConfigMultiMapping().\ set_global([a, b]).\ set_object_type(nn.Conv2d, [c, d])

how would the results be ordered in the resulting loggers? I think any reasonable order is fine, I'm just asking what the actual order would be. We should probably document it.

i'd probably rather split this consideration into a seperate PR because if we're going to have guarantees about ordering we'd probably want a more robust test suite for it. There's no explicit ordering considerations at the moment, thoguh I assume it'd be ordered as one would expected, i.e. fifo for qconfigs.

makes sense, it seems that for anything other than testing one qconfig an order would be required in order to interpret the results correctly

Summary: Added QConfigMultiMapping which is essentially a List[QConfigMapping] with set methods and dedicated handling to avoid unwanted matches and improve UX. note: the from __future__ import annotations line caused weird errors when the QConfigMultiMapping class was put in _numeric_suite_fx.py so it was moved. Test Plan: python test/test_quantization.py TestFxNumericSuiteNShadows Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

Summary: Added QConfigMultiMapping which is essentially a List[QConfigMapping] with set methods and dedicated handling to avoid unwanted matches and improve UX. note: the from __future__ import annotations line caused weird errors when the QConfigMultiMapping class was put in _numeric_suite_fx.py so it was moved. Test Plan: python test/test_quantization.py TestFxNumericSuiteNShadows Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 5c36a3899aa0c305e09909471a5f6bc81a8b9387 Pull Request resolved: #86922

vkuzo · 2022-10-24T22:26:31Z

torch/ao/ns/_numeric_suite_fx.py

@@ -753,7 +754,7 @@ def extend_logger_results_with_comparison(
 def prepare_n_shadows_model(
    model: torch.nn.Module,
    example_inputs: Any,
-    qconfig_mappings: List[QConfigMapping],
+    qconfig_mappings: QConfigMultiMapping,


nit: update var name?

vkuzo

thanks for working on this!

Summary: Added QConfigMultiMapping which is essentially a List[QConfigMapping] with set methods and dedicated handling to avoid unwanted matches and improve UX. note: the from __future__ import annotations line caused weird errors when the QConfigMultiMapping class was put in _numeric_suite_fx.py so it was moved. Test Plan: python test/test_quantization.py TestFxNumericSuiteNShadows Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

Summary: Added QConfigMultiMapping which is essentially a List[QConfigMapping] with set methods and dedicated handling to avoid unwanted matches and improve UX. note: the from __future__ import annotations line caused weird errors when the QConfigMultiMapping class was put in _numeric_suite_fx.py so it was moved. Test Plan: python test/test_quantization.py TestFxNumericSuiteNShadows Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: a16140056210efb177e3d15684f11b8b23db1cb1 Pull Request resolved: #86922

Summary: Added QConfigMultiMapping which is essentially a List[QConfigMapping] with set methods and dedicated handling to avoid unwanted matches and improve UX. note: the from __future__ import annotations line caused weird errors when the QConfigMultiMapping class was put in _numeric_suite_fx.py so it was moved. Test Plan: python test/test_quantization.py TestFxNumericSuiteNShadows Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

Summary: Added QConfigMultiMapping which is essentially a List[QConfigMapping] with set methods and dedicated handling to avoid unwanted matches and improve UX. note: the from __future__ import annotations line caused weird errors when the QConfigMultiMapping class was put in _numeric_suite_fx.py so it was moved. Test Plan: python test/test_quantization.py TestFxNumericSuiteNShadows Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 2edc233e2e49fa96801f726f37062ded5ec037a7 Pull Request resolved: #86922

Summary: Added QConfigMultiMapping which is essentially a List[QConfigMapping] with set methods and dedicated handling to avoid unwanted matches and improve UX. note: the from __future__ import annotations line caused weird errors when the QConfigMultiMapping class was put in _numeric_suite_fx.py so it was moved. Test Plan: python test/test_quantization.py TestFxNumericSuiteNShadows Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

Summary: Added QConfigMultiMapping which is essentially a List[QConfigMapping] with set methods and dedicated handling to avoid unwanted matches and improve UX. note: the from __future__ import annotations line caused weird errors when the QConfigMultiMapping class was put in _numeric_suite_fx.py so it was moved. Test Plan: python test/test_quantization.py TestFxNumericSuiteNShadows Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: a3e4401ce382f11d2034207fcb85ad78e0e0c1fc Pull Request resolved: #86922

Summary: Added QConfigMultiMapping which is essentially a List[QConfigMapping] with set methods and dedicated handling to avoid unwanted matches and improve UX. note: the from __future__ import annotations line caused weird errors when the QConfigMultiMapping class was put in _numeric_suite_fx.py so it was moved. Test Plan: python test/test_quantization.py TestFxNumericSuiteNShadows Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

Summary: Added QConfigMultiMapping which is essentially a List[QConfigMapping] with set methods and dedicated handling to avoid unwanted matches and improve UX. note: the from __future__ import annotations line caused weird errors when the QConfigMultiMapping class was put in _numeric_suite_fx.py so it was moved. Test Plan: python test/test_quantization.py TestFxNumericSuiteNShadows Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 85875573b3df1d9df9c6350ce75d025831ffedf4 Pull Request resolved: #86922

Summary: Added QConfigMultiMapping which is essentially a List[QConfigMapping] with set methods and dedicated handling to avoid unwanted matches and improve UX. note: the from __future__ import annotations line caused weird errors when the QConfigMultiMapping class was put in _numeric_suite_fx.py so it was moved. Test Plan: python test/test_quantization.py TestFxNumericSuiteNShadows Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

Summary: Added QConfigMultiMapping which is essentially a List[QConfigMapping] with set methods and dedicated handling to avoid unwanted matches and improve UX. note: the from __future__ import annotations line caused weird errors when the QConfigMultiMapping class was put in _numeric_suite_fx.py so it was moved. Test Plan: python test/test_quantization.py TestFxNumericSuiteNShadows Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: a145d0675db6d64f4e8c1b62580eb7e7df75db80 Pull Request resolved: #86922

HDCharles · 2022-10-26T18:52:11Z

@pytorchmergebot merge

pytorchmergebot · 2022-10-26T18:56:49Z

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

github-actions · 2022-10-26T18:57:31Z

Hey @HDCharles.
You've committed this PR, but it does not have both a 'release notes: ...' and 'topics: ...' label. Please add one of each to the PR. The 'release notes: ...' label should represent the part of PyTorch that this PR changes (fx, autograd, distributed, etc) and the 'topics: ...' label should represent the kind of PR it is (not user facing, new feature, bug fix, perf improvement, etc). The list of valid labels can be found here for the 'release notes: ...' and here for the 'topics: ...'.
For changes that are 'topic: not user facing' there is no need for a release notes label.

Summary: Added QConfigMultiMapping which is essentially a List[QConfigMapping] with set methods and dedicated handling to avoid unwanted matches and improve UX. note: the from __future__ import annotations line caused weird errors when the QConfigMultiMapping class was put in _numeric_suite_fx.py so it was moved. Test Plan: python test/test_quantization.py TestFxNumericSuiteNShadows Reviewers: Subscribers: Tasks: Tags: Pull Request resolved: pytorch#86922 Approved by: https://github.com/vkuzo

[wip][ao][ns] Replacing List[QConfigMapping] in PNP

60a5ce2

Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

HDCharles requested review from jerryzh168 and z-a-f as code owners October 13, 2022 18:00

pytorch-bot bot added the release notes: quantization release notes category label Oct 13, 2022

HDCharles added a commit that referenced this pull request Oct 13, 2022

[wip][ao][ns] Replacing List[QConfigMapping] in PNP

1f8d803

Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 93cf9ae0015176e092d6589ec04d4526131213ad Pull Request resolved: #86922

Update on "[wip][ao][ns] Replacing List[QConfigMapping] in PNP"

1e5fcbb

Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

HDCharles added a commit that referenced this pull request Oct 17, 2022

[wip][ao][ns] Replacing List[QConfigMapping] in PNP

9928a14

Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 3896ee91b73961b32f7c3d103717436ba8ae9b4f Pull Request resolved: #86922

Update on "[wip][ao][ns] Replacing List[QConfigMapping] in PNP"

33fe567

Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

HDCharles added a commit that referenced this pull request Oct 17, 2022

[wip][ao][ns] Replacing List[QConfigMapping] in PNP

4403854

Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 92353bd57a26786fdec554aba5233c2273ac2e16 Pull Request resolved: #86922

Update on "[wip][ao][ns] Replacing List[QConfigMapping] in PNP"

a0433cd

Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

HDCharles changed the title ~~[wip][ao][ns] Replacing List[QConfigMapping] in PNP~~ [ao][ns] Replacing List[QConfigMapping] in PNP Oct 18, 2022

HDCharles requested a review from vkuzo October 24, 2022 17:10

vkuzo reviewed Oct 24, 2022

View reviewed changes

vkuzo approved these changes Oct 24, 2022

View reviewed changes

pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Oct 24, 2022

pytorchmergebot added the Merged label Oct 26, 2022

pytorchmergebot closed this in f7a04f3 Oct 26, 2022

HDCharles added the topic: improvements topic category label Oct 26, 2022

facebook-github-bot deleted the gh/HDCharles/119/head branch June 8, 2023 14:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ao][ns] Replacing List[QConfigMapping] in PNP #86922

[ao][ns] Replacing List[QConfigMapping] in PNP #86922

HDCharles commented Oct 13, 2022 •

edited

pytorch-bot bot commented Oct 13, 2022 •

edited

vkuzo Oct 24, 2022

HDCharles Oct 24, 2022

vkuzo Oct 24, 2022

vkuzo Oct 24, 2022

HDCharles Oct 24, 2022

vkuzo Oct 24, 2022

HDCharles Oct 24, 2022

vkuzo Oct 24, 2022

vkuzo Oct 24, 2022

vkuzo left a comment

HDCharles commented Oct 26, 2022

pytorchmergebot commented Oct 26, 2022

github-actions bot commented Oct 26, 2022

[ao][ns] Replacing List[QConfigMapping] in PNP #86922

[ao][ns] Replacing List[QConfigMapping] in PNP #86922

Conversation

HDCharles commented Oct 13, 2022 • edited

pytorch-bot bot commented Oct 13, 2022 • edited

🔗 Helpful Links

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

✅ No Failures

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vkuzo left a comment

Choose a reason for hiding this comment

HDCharles commented Oct 26, 2022

pytorchmergebot commented Oct 26, 2022

Merge started

github-actions bot commented Oct 26, 2022

HDCharles commented Oct 13, 2022 •

edited

pytorch-bot bot commented Oct 13, 2022 •

edited