[Quant][fx][bc-breaking] Make convert.py smaller #90189

andrewor14 · 2022-12-05T16:41:24Z

Stack from ghstack (oldest at bottom):

-> [Quant][fx][bc-breaking] Make convert.py smaller #90189

Summary: This commit moves helper functions that are not core
to the convert logic out of convert.py, which was more than
1000 lines. This helps with readability since a new developer
won't have to scroll through hundreds of lines of util functions
to understand the core logic. There should be no change in
functionality in this commit.

BC-breaking notes: The following helper functions that were
previously exposed under the torch.ao.quantization.fx.convert
namespace are now made private. Many of these are moved to the
new convert_utils.py

convert_custom_module
convert_standalone_module
convert_weighted_module
get_module_path_and_prefix,
has_none_qconfig,
insert_dequantize_node,
is_conversion_supported,
maybe_recursive_remove_dequantize,
replace_observer_or_dequant_stub_with_dequantize_node,
restore_state,
run_weight_observers,

Test Plan:
python test/test_quantization.py TestQuantizeFx
python test/test_quantization.py TestQuantizeFxOps

Reviewers: jerryzh168, vkuzo

Subscribers: jerryzh168, vkuzo

Summary: This commit moves helper functions that are not core to the convert logic out of convert.py, which was more than 1000 lines. This helps with readability since a new developer won't have to scroll through hundreds of lines of util functions to understand the core logic. There should be no change in functionality in this commit. BC-breaking notes: The following helper functions that were previously exposed under the `torch.ao.quantization.fx.convert` namespace are now made private. Many of these are moved to the new convert_utils.py ``` convert_custom_module convert_standalone_module convert_weighted_module get_module_path_and_prefix, has_none_qconfig, insert_dequantize_node, is_conversion_supported, maybe_recursive_remove_dequantize, replace_observer_or_dequant_stub_with_dequantize_node, restore_state, run_weight_observers, ``` Test Plan: python test/test_quantization.py TestQuantizeFx python test/test_quantization.py TestQuantizeFxOps Reviewers: jerryzh168, vkuzo Subscribers: jerryzh168, vkuzo [ghstack-poisoned]

pytorch-bot · 2022-12-05T16:41:26Z

🔗 Helpful Links

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

📄 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 73549c0:
💚 Looks good so far! There are no failures yet. 💚

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

Summary: This commit moves helper functions that are not core to the convert logic out of convert.py, which was more than 1000 lines. This helps with readability since a new developer won't have to scroll through hundreds of lines of util functions to understand the core logic. There should be no change in functionality in this commit. BC-breaking notes: The following helper functions that were previously exposed under the `torch.ao.quantization.fx.convert` namespace are now made private. Many of these are moved to the new convert_utils.py ``` convert_custom_module convert_standalone_module convert_weighted_module get_module_path_and_prefix, has_none_qconfig, insert_dequantize_node, is_conversion_supported, maybe_recursive_remove_dequantize, replace_observer_or_dequant_stub_with_dequantize_node, restore_state, run_weight_observers, ``` Test Plan: python test/test_quantization.py TestQuantizeFx python test/test_quantization.py TestQuantizeFxOps Reviewers: jerryzh168, vkuzo Subscribers: jerryzh168, vkuzo [ghstack-poisoned]

Summary: This commit moves helper functions that are not core to the convert logic out of convert.py, which was more than 1000 lines. This helps with readability since a new developer won't have to scroll through hundreds of lines of util functions to understand the core logic. There should be no change in functionality in this commit. BC-breaking notes: The following helper functions that were previously exposed under the `torch.ao.quantization.fx.convert` namespace are now made private. Many of these are moved to the new convert_utils.py ``` convert_custom_module convert_standalone_module convert_weighted_module get_module_path_and_prefix, has_none_qconfig, insert_dequantize_node, is_conversion_supported, maybe_recursive_remove_dequantize, replace_observer_or_dequant_stub_with_dequantize_node, restore_state, run_weight_observers, ``` Test Plan: python test/test_quantization.py TestQuantizeFx python test/test_quantization.py TestQuantizeFxOps Reviewers: jerryzh168, vkuzo Subscribers: jerryzh168, vkuzo ghstack-source-id: baf3cf519a11fbedcb6a540c735ba58c758a79ae Pull Request resolved: #90189

Summary: This commit moves helper functions that are not core to the convert logic out of convert.py, which was more than 1000 lines. This helps with readability since a new developer won't have to scroll through hundreds of lines of util functions to understand the core logic. There should be no change in functionality in this commit. BC-breaking notes: The following helper functions that were previously exposed under the `torch.ao.quantization.fx.convert` namespace are now made private. Many of these are moved to the new convert_utils.py ``` convert_custom_module convert_standalone_module convert_weighted_module get_module_path_and_prefix, has_none_qconfig, insert_dequantize_node, is_conversion_supported, maybe_recursive_remove_dequantize, replace_observer_or_dequant_stub_with_dequantize_node, restore_state, run_weight_observers, ``` Test Plan: python test/test_quantization.py TestQuantizeFx python test/test_quantization.py TestQuantizeFxOps Reviewers: jerryzh168, vkuzo Subscribers: jerryzh168, vkuzo [ghstack-poisoned]

Summary: This commit moves helper functions that are not core to the convert logic out of convert.py, which was more than 1000 lines. This helps with readability since a new developer won't have to scroll through hundreds of lines of util functions to understand the core logic. There should be no change in functionality in this commit. BC-breaking notes: The following helper functions that were previously exposed under the `torch.ao.quantization.fx.convert` namespace are now made private. Many of these are moved to the new convert_utils.py ``` convert_custom_module convert_standalone_module convert_weighted_module get_module_path_and_prefix, has_none_qconfig, insert_dequantize_node, is_conversion_supported, maybe_recursive_remove_dequantize, replace_observer_or_dequant_stub_with_dequantize_node, restore_state, run_weight_observers, ``` Test Plan: python test/test_quantization.py TestQuantizeFx python test/test_quantization.py TestQuantizeFxOps Reviewers: jerryzh168, vkuzo Subscribers: jerryzh168, vkuzo ghstack-source-id: 7cb406e562e76d64d4071b5f5f029ab5f2a09b7b Pull Request resolved: #90189

Summary: This commit moves helper functions that are not core to the convert logic out of convert.py, which was more than 1000 lines. This helps with readability since a new developer won't have to scroll through hundreds of lines of util functions to understand the core logic. There should be no change in functionality in this commit. BC-breaking notes: The following helper functions that were previously exposed under the `torch.ao.quantization.fx.convert` namespace are now made private. Many of these are moved to the new convert_utils.py ``` convert_custom_module convert_standalone_module convert_weighted_module get_module_path_and_prefix, has_none_qconfig, insert_dequantize_node, is_conversion_supported, maybe_recursive_remove_dequantize, replace_observer_or_dequant_stub_with_dequantize_node, restore_state, run_weight_observers, ``` Test Plan: python test/test_quantization.py TestQuantizeFx python test/test_quantization.py TestQuantizeFxOps Reviewers: jerryzh168, vkuzo Subscribers: jerryzh168, vkuzo [ghstack-poisoned]

Summary: This commit moves helper functions that are not core to the convert logic out of convert.py, which was more than 1000 lines. This helps with readability since a new developer won't have to scroll through hundreds of lines of util functions to understand the core logic. There should be no change in functionality in this commit. BC-breaking notes: The following helper functions that were previously exposed under the `torch.ao.quantization.fx.convert` namespace are now made private. Many of these are moved to the new convert_utils.py ``` convert_custom_module convert_standalone_module convert_weighted_module get_module_path_and_prefix, has_none_qconfig, insert_dequantize_node, is_conversion_supported, maybe_recursive_remove_dequantize, replace_observer_or_dequant_stub_with_dequantize_node, restore_state, run_weight_observers, ``` Test Plan: python test/test_quantization.py TestQuantizeFx python test/test_quantization.py TestQuantizeFxOps Reviewers: jerryzh168, vkuzo Subscribers: jerryzh168, vkuzo ghstack-source-id: f1eeb74d117defd5d278163b9463f12c14f0f192 Pull Request resolved: #90189

pytorch-bot · 2022-12-07T01:04:05Z

❌ 🤖 pytorchbot command failed:

@pytorchbot: error: argument command: invalid choice: 'unmerge' (choose from 'merge', 'revert', 'rebase', 'label', 'drci')

usage: @pytorchbot [-h] {merge,revert,rebase,label,drci} ...

Try @pytorchbot --help for more info.

andrewor14 · 2022-12-07T01:04:47Z

Accidentally tried to merge this, reopening

pytorchmergebot · 2022-12-07T01:05:21Z

Merge failed

Reason: Comment with id 1340231722 not found

Details for Dev Infra team

Raised by workflow job

jerryzh168 · 2022-12-07T03:16:47Z

wondering if we need to call this bc-breaking? are helper functions under torch.ao.quantization.convert file also public apis?

andrewor14 · 2022-12-07T16:12:52Z

wondering if we need to call this bc-breaking? are helper functions under torch.ao.quantization.convert file also public apis?

I guess they're technically public, but yes this is definitely less bc-breaking than say adding example_inputs

andrewor14 · 2022-12-07T16:13:03Z

@pytorchbot merge

pytorchmergebot · 2022-12-07T16:16:20Z

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

seemethere · 2022-12-08T00:49:33Z

@pytorchbot revert -c ghfirst -m "Fails internal tests due to potential circular import, see https://www.internalfb.com/diff/D41817429?dst_version_fbid=1453307181865235&transaction_fbid=899728221278938"

pytorchmergebot · 2022-12-08T00:51:05Z

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

pytorchmergebot · 2022-12-08T00:51:17Z

@andrewor14 your PR has been successfully reverted.

This reverts commit 824641b. Reverted #90189 on behalf of https://github.com/seemethere due to Fails internal tests due to potential circular import, see https://www.internalfb.com/diff/D41817429?dst_version_fbid=1453307181865235&transaction_fbid=899728221278938

Summary: This commit moves helper functions that are not core to the convert logic out of convert.py, which was more than 1000 lines. This helps with readability since a new developer won't have to scroll through hundreds of lines of util functions to understand the core logic. There should be no change in functionality in this commit. BC-breaking notes: The following helper functions that were previously exposed under the `torch.ao.quantization.fx.convert` namespace are now made private. Many of these are moved to the new convert_utils.py ``` convert_custom_module convert_standalone_module convert_weighted_module get_module_path_and_prefix, has_none_qconfig, insert_dequantize_node, is_conversion_supported, maybe_recursive_remove_dequantize, replace_observer_or_dequant_stub_with_dequantize_node, restore_state, run_weight_observers, ``` Test Plan: python test/test_quantization.py TestQuantizeFx python test/test_quantization.py TestQuantizeFxOps Reviewers: jerryzh168, vkuzo Subscribers: jerryzh168, vkuzo Pull Request resolved: pytorch#90189 Approved by: https://github.com/jerryzh168

…)" This reverts commit 824641b. Reverted pytorch#90189 on behalf of https://github.com/seemethere due to Fails internal tests due to potential circular import, see https://www.internalfb.com/diff/D41817429?dst_version_fbid=1453307181865235&transaction_fbid=899728221278938

pytorch-bot bot added the release notes: AO frontend label Dec 5, 2022

github-actions bot added the release notes: quantization release notes category label Dec 5, 2022

andrewor14 requested review from jerryzh168 and vkuzo December 5, 2022 16:50

andrewor14 added the topic: bc breaking topic category label Dec 5, 2022

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

andrewor14 closed this Dec 7, 2022

andrewor14 reopened this Dec 7, 2022

jerryzh168 approved these changes Dec 7, 2022

View reviewed changes

pytorchmergebot added the Merged label Dec 7, 2022

pytorchmergebot closed this in 824641b Dec 7, 2022

pytorchmergebot added the Reverted label Dec 8, 2022

facebook-github-bot deleted the gh/andrewor14/39/head branch June 8, 2023 15:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Quant][fx][bc-breaking] Make convert.py smaller #90189

[Quant][fx][bc-breaking] Make convert.py smaller #90189

andrewor14 commented Dec 5, 2022 •

edited

Loading

pytorch-bot bot commented Dec 5, 2022 •

edited

Loading

pytorch-bot bot commented Dec 7, 2022

andrewor14 commented Dec 7, 2022

pytorchmergebot commented Dec 7, 2022

jerryzh168 commented Dec 7, 2022

andrewor14 commented Dec 7, 2022

andrewor14 commented Dec 7, 2022

pytorchmergebot commented Dec 7, 2022

seemethere commented Dec 8, 2022

pytorchmergebot commented Dec 8, 2022

pytorchmergebot commented Dec 8, 2022

[Quant][fx][bc-breaking] Make convert.py smaller #90189

[Quant][fx][bc-breaking] Make convert.py smaller #90189

Conversation

andrewor14 commented Dec 5, 2022 • edited Loading

pytorch-bot bot commented Dec 5, 2022 • edited Loading

🔗 Helpful Links

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

✅ No Failures

pytorch-bot bot commented Dec 7, 2022

andrewor14 commented Dec 7, 2022

pytorchmergebot commented Dec 7, 2022

Merge failed

jerryzh168 commented Dec 7, 2022

andrewor14 commented Dec 7, 2022

andrewor14 commented Dec 7, 2022

pytorchmergebot commented Dec 7, 2022

Merge started

seemethere commented Dec 8, 2022

pytorchmergebot commented Dec 8, 2022

pytorchmergebot commented Dec 8, 2022

andrewor14 commented Dec 5, 2022 •

edited

Loading

pytorch-bot bot commented Dec 5, 2022 •

edited

Loading