Skip to content

[quant] Fix the quant_min/quant_max override in FakeQuantize #74581

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.

Sign up for GitHub

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

Jump to bottom
Closed
LiamZhuuu wants to merge 1 commit into from
Closed

[quant] Fix the quant_min/quant_max override in FakeQuantize #74581

LiamZhuuu wants to merge 1 commit into from

Conversation

@LiamZhuuu
Copy link

@LiamZhuuu LiamZhuuu commented Mar 22, 2022

Summary: As title, currently the quant_min/quant_max of the FakeQuantize are not passed to the observer.

Test Plan:

[jiaxuzhu@devvm3400.frc0 /data/users/jiaxuzhu/fbsource/fbcode] buck test mode/dev //caffe2/test:quantization -- --exact 'caffe2/test:quantization - test_quant_min_max_override (quantization.core.test_workflow_module.TestFakeQuantize)'
Restarting Buck daemon because Buck version has changed...
Buck daemon started.
DEBUG: /data/users/jiaxuzhu/fbsource/tools/build_defs/fbcode_macros/build_defs/lib/cpp_common.bzl:287:14: Using disallowed linker flag 'ANativeActivity_onCreate' in library rule 'fbsource//third-party/toolchains/android-ndk:r18b_native_app_glue'
DEBUG: /data/users/jiaxuzhu/fbsource/tools/build_defs/fbcode_macros/build_defs/lib/cpp_common.bzl:287:14: Using disallowed linker flag 'arvr/third-party/toolchains/platform009/build/mesa/lib/libGL.so' in library rule 'fbsource//third-party/toolchains:opengl'
DEBUG: /data/users/jiaxuzhu/fbsource/tools/build_defs/fbcode_macros/build_defs/lib/cpp_common.bzl:287:14: Using disallowed linker flag 'arvr/third-party/freeglut/3.0.0/libs/x64-linux/libglut.a' in library rule 'fbsource//third-party/toolchains:GLUT'
Parsing buck files: finished in 20.4 sec
Creating action graph: finished in 44.0 sec
[RE] Metadata: Session ID=[https://fburl.com/b/reSessionID-bafd60fc-6119-4ae7-95b5-7e36b783310b]
[RE] Waiting on 0 remote actions. Completed 144 actions remotely, action cache hit rate: 94.59%.
Downloaded 10199/10203 artifacts, 1.08 Gbytes, 0.0% cache miss (for updated rules)
Building: finished in 02:10.2 min (100%) 21299/21299 jobs, 21299/21299 updated
  Total time: 03:14.7 min
More details at https://www.internalfb.com/intern/buck/build/a2325f91-fc53-40ec-bec1-d7bd5c931b0f
BUILD SUCCEEDED
Tpx test run coordinator for Facebook. See https://fburl.com/tpx for details.
Running with tpx session id: e1dd4647-9f9c-4f63-8390-6c13f3ebf7a1
Trace available for this run at /tmp/tpx-20220322-135414.576062-e1dd4647-9f9c-4f63-8390-6c13f3ebf7a1/trace.log
RemoteExecution session id: reSessionID-e1dd4647-9f9c-4f63-8390-6c13f3ebf7a1-tpx
Started reporting to test run: https://www.internalfb.com/intern/testinfra/testrun/6473924546262193
    ✓ ListingSuccess: caffe2/test:quantization : 483 tests discovered (31.784)
    ✓ Pass: caffe2/test:quantization - test_quant_min_max_override (quantization.core.test_workflow_module.TestFakeQuantize) (19.030)
Summary
  Pass: 1
  ListingSuccess: 1
If you need help understanding your runs, please follow the wiki: https://fburl.com/posting_in_tpx_users
Finished test run: https://www.internalfb.com/intern/testinfra/testrun/6473924546262193

Differential Revision: D34971236

@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Mar 22, 2022 •
edited
Loading

🔗 Helpful links

💊 CI failures summary and remediations

As of commit f19105e (more details on the Dr. CI page):

Commit f19105e was recently pushed. Waiting for builds...

This comment was automatically generated by Dr. CI (expand for details).

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Mar 22, 2022

This pull request was exported from Phabricator. Differential Revision: D34971236

1 similar comment
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Mar 23, 2022

This pull request was exported from Phabricator. Differential Revision: D34971236

jerryzh168
jerryzh168 reviewed Mar 23, 2022
View reviewed changes
test/quantization/core/test_workflow_module.py Outdated
Comment on lines +768 to +770
Copy link
Contributor

@jerryzh168 jerryzh168 Mar 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why the override numbers does not match?

Copy link
Contributor

@jerryzh168 jerryzh168 Mar 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add a TODO for us to fix it, user should be able to override the values..

jerryzh168
jerryzh168 reviewed Mar 23, 2022
View reviewed changes
torch/ao/quantization/fake_quantize.py Outdated
Comment on lines 126 to 127
Copy link
Contributor

@jerryzh168 jerryzh168 Mar 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we move these things after the initialization of activation_post_process and assign self.quant_min = self.activation_post_process.quant_min etc.?

@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Mar 23, 2022

This pull request was exported from Phabricator. Differential Revision: D34971236

@jiaxuzhu92 @facebook-github-bot
[quant] Populate FakeQuantize quant_min/quant_max to observer (pytorc…
f19105e 
…h#74581)

Summary:
Pull Request resolved: pytorch#74581

As title, currently the quant_min/quant_max of the FakeQuantize are not populated to the observer. We plan to populate when they are both not None.

To do this we need to do
1. Remove the current default quant_min/quant_max value (0/255) as it's not universal for various dtype.
2. Move the upper bound/lower bound check before creating the observer.

Test Plan:
```
[jiaxuzhu@devvm3400.frc0 /data/users/jiaxuzhu/fbsource/fbcode] buck test mode/dev //caffe2/test:quantization -- --exact 'caffe2/test:quantization - test_quant_min_max_override (quantization.core.test_workflow_module.TestFakeQuantize)'
Parsing buck files: finished in 0.8 sec
Downloaded 0/2 artifacts, 0.00 bytes, 100.0% cache miss (for updated rules)
Building: finished in 9.5 sec (100%) 18535/84579 jobs, 2/84579 updated
  Total time: 10.3 sec
More details at https://www.internalfb.com/intern/buck/build/1cab97ef-0788-4d06-92ed-a828995e3bde
BUILD SUCCEEDED
Tpx test run coordinator for Facebook. See https://fburl.com/tpx for details.
Running with tpx session id: 24be645e-eebc-45d6-8111-052ef1225fa0
Trace available for this run at /tmp/tpx-20220323-094106.724238-24be645e-eebc-45d6-8111-052ef1225fa0/trace.log
RemoteExecution session id: reSessionID-24be645e-eebc-45d6-8111-052ef1225fa0-tpx
Started reporting to test run: https://www.internalfb.com/intern/testinfra/testrun/5066549674998735
    ✓ ListingSuccess: caffe2/test:quantization : 483 tests discovered (20.179)
    ✓ Pass: caffe2/test:quantization - test_quant_min_max_override (quantization.core.test_workflow_module.TestFakeQuantize) (18.896)
Summary
  Pass: 1
  ListingSuccess: 1
If you need help understanding your runs, please follow the wiki: https://fburl.com/posting_in_tpx_users
Finished test run: https://www.internalfb.com/intern/testinfra/testrun/5066549674998735
```

Reviewed By: jerryzh168

Differential Revision: D34971236

fbshipit-source-id: f7f0b252e3174ae91c85709b7ce07528eb7ff571
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Mar 24, 2022

This pull request was exported from Phabricator. Differential Revision: D34971236

facebook-github-bot pushed a commit that referenced this pull request Mar 24, 2022
@jiaxuzhu92 @facebook-github-bot
[quant] Populate FakeQuantize quant_min/quant_max to observer (#74581)
f6980bc 
Summary:
Pull Request resolved: #74581

As title, currently the quant_min/quant_max of the FakeQuantize are not populated to the observer. We plan to populate when they are both not None.

To do this we need to do
1. Remove the current default quant_min/quant_max value (0/255) as it's not universal for various dtype.
2. Move the upper bound/lower bound check before creating the observer.

Test Plan:
```
[jiaxuzhu@devvm3400.frc0 /data/users/jiaxuzhu/fbsource/fbcode] buck test mode/dev //caffe2/test:quantization -- --exact 'caffe2/test:quantization - test_quant_min_max_override (quantization.core.test_workflow_module.TestFakeQuantize)'
Parsing buck files: finished in 0.8 sec
Downloaded 0/2 artifacts, 0.00 bytes, 100.0% cache miss (for updated rules)
Building: finished in 9.5 sec (100%) 18535/84579 jobs, 2/84579 updated
  Total time: 10.3 sec
More details at https://www.internalfb.com/intern/buck/build/1cab97ef-0788-4d06-92ed-a828995e3bde
BUILD SUCCEEDED
Tpx test run coordinator for Facebook. See https://fburl.com/tpx for details.
Running with tpx session id: 24be645e-eebc-45d6-8111-052ef1225fa0
Trace available for this run at /tmp/tpx-20220323-094106.724238-24be645e-eebc-45d6-8111-052ef1225fa0/trace.log
RemoteExecution session id: reSessionID-24be645e-eebc-45d6-8111-052ef1225fa0-tpx
Started reporting to test run: https://www.internalfb.com/intern/testinfra/testrun/5066549674998735
    ✓ ListingSuccess: caffe2/test:quantization : 483 tests discovered (20.179)
    ✓ Pass: caffe2/test:quantization - test_quant_min_max_override (quantization.core.test_workflow_module.TestFakeQuantize) (18.896)
Summary
  Pass: 1
  ListingSuccess: 1
If you need help understanding your runs, please follow the wiki: https://fburl.com/posting_in_tpx_users
Finished test run: https://www.internalfb.com/intern/testinfra/testrun/5066549674998735
```

Reviewed By: jerryzh168

Differential Revision: D34971236

fbshipit-source-id: 4407fd03116a296053256b333f7ce6d28dcc9c42
@github-actions
Copy link
Contributor

github-actions bot commented Mar 24, 2022

Hey @LiamZhuuu.
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.

shahofblah pushed a commit that referenced this pull request Mar 25, 2022
@jiaxuzhu92 @shahofblah
[quant] Populate FakeQuantize quant_min/quant_max to observer (#74581)
ca01442 
Summary:
Pull Request resolved: #74581

As title, currently the quant_min/quant_max of the FakeQuantize are not populated to the observer. We plan to populate when they are both not None.

To do this we need to do
1. Remove the current default quant_min/quant_max value (0/255) as it's not universal for various dtype.
2. Move the upper bound/lower bound check before creating the observer.

Test Plan:
```
[jiaxuzhu@devvm3400.frc0 /data/users/jiaxuzhu/fbsource/fbcode] buck test mode/dev //caffe2/test:quantization -- --exact 'caffe2/test:quantization - test_quant_min_max_override (quantization.core.test_workflow_module.TestFakeQuantize)'
Parsing buck files: finished in 0.8 sec
Downloaded 0/2 artifacts, 0.00 bytes, 100.0% cache miss (for updated rules)
Building: finished in 9.5 sec (100%) 18535/84579 jobs, 2/84579 updated
  Total time: 10.3 sec
More details at https://www.internalfb.com/intern/buck/build/1cab97ef-0788-4d06-92ed-a828995e3bde
BUILD SUCCEEDED
Tpx test run coordinator for Facebook. See https://fburl.com/tpx for details.
Running with tpx session id: 24be645e-eebc-45d6-8111-052ef1225fa0
Trace available for this run at /tmp/tpx-20220323-094106.724238-24be645e-eebc-45d6-8111-052ef1225fa0/trace.log
RemoteExecution session id: reSessionID-24be645e-eebc-45d6-8111-052ef1225fa0-tpx
Started reporting to test run: https://www.internalfb.com/intern/testinfra/testrun/5066549674998735
    ✓ ListingSuccess: caffe2/test:quantization : 483 tests discovered (20.179)
    ✓ Pass: caffe2/test:quantization - test_quant_min_max_override (quantization.core.test_workflow_module.TestFakeQuantize) (18.896)
Summary
  Pass: 1
  ListingSuccess: 1
If you need help understanding your runs, please follow the wiki: https://fburl.com/posting_in_tpx_users
Finished test run: https://www.internalfb.com/intern/testinfra/testrun/5066549674998735
```

Reviewed By: jerryzh168

Differential Revision: D34971236

fbshipit-source-id: 4407fd03116a296053256b333f7ce6d28dcc9c42
(cherry picked from commit f6980bc)
@WBobby WBobby mentioned this pull request Aug 17, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jerryzh168 jerryzh168 jerryzh168 approved these changes

Assignees

No one assigned

Labels

cla signed fb-exported

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@LiamZhuuu @facebook-github-bot @jerryzh168 @jiaxuzhu92