Skip to content
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.

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

[quant][graphmode][fx] Merge all quantization mode #45292

Closed
wants to merge 21 commits into from

Conversation

jerryzh168
Copy link
Contributor

@jerryzh168 jerryzh168 commented Sep 24, 2020

Stack from ghstack:

Summary:
This PR merges all quantization mode and will only expose the following top level functions:

prepare_fx
prepare_qat_fx
convert_fx

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: D23913105

Summary:
This PR merges all quantization mode and will only expose the following top level functions:
```
prepare_fx
prepare_qat_fx
convert_fx
```

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

[ghstack-poisoned]
jerryzh168 added a commit that referenced this pull request Sep 24, 2020
Summary:
This PR merges all quantization mode and will only expose the following top level functions:
```
prepare_fx
prepare_qat_fx
convert_fx
```

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

ghstack-source-id: 13d0aa89c93df0ba0707deeedecc5350e146dc62
Pull Request resolved: #45292
@dr-ci
Copy link

dr-ci bot commented Sep 24, 2020

💊 CI failures summary and remediations

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


Commit 1d191e0 was recently pushed. Waiting for builds...


This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions on the GitHub issue tracker or post in the (internal) Dr. CI Users group.

See how this bot performed.

This comment has been revised 109 times.

Summary:
This PR merges all quantization mode and will only expose the following top level functions:
```
prepare_fx
prepare_qat_fx
convert_fx
```

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: [D23913105](https://our.internmc.facebook.com/intern/diff/D23913105)

[ghstack-poisoned]
Summary:
This PR merges all quantization mode and will only expose the following top level functions:
```
prepare_fx
prepare_qat_fx
convert_fx
```

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: [D23913105](https://our.internmc.facebook.com/intern/diff/D23913105)

[ghstack-poisoned]
Summary:
This PR merges all quantization mode and will only expose the following top level functions:
```
prepare_fx
prepare_qat_fx
convert_fx
```

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: [D23913105](https://our.internmc.facebook.com/intern/diff/D23913105)

[ghstack-poisoned]
Copy link
Contributor

@vkuzo vkuzo left a comment

Choose a reason for hiding this comment

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

looks awesome, the new names are much easier to read!

@@ -138,3 +138,38 @@ def get_next_qparams_idx(module, qparams):
qparam_full_path = key + str(idx)
inputs.append(graph.create_node('get_attr', qparam_full_path))
return graph.create_node('call_function', quantize_op, tuple(inputs), {})

def activation_is_dynamically_quantized(qconfig):
Copy link
Contributor

Choose a reason for hiding this comment

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

looks super clean, thanks!

Summary:
This PR merges all quantization mode and will only expose the following top level functions:
```
prepare_fx
prepare_qat_fx
convert_fx
```

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: [D23913105](https://our.internmc.facebook.com/intern/diff/D23913105)

[ghstack-poisoned]
jerryzh168 added a commit that referenced this pull request Sep 25, 2020
Summary:
This PR merges all quantization mode and will only expose the following top level functions:
```
prepare_fx
prepare_qat_fx
convert_fx
```

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

ghstack-source-id: 264ca59e1e9255e9e5ca08194101e6f2ccc10af6
Pull Request resolved: #45292
Summary:
This PR merges all quantization mode and will only expose the following top level functions:
```
prepare_fx
prepare_qat_fx
convert_fx
```

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: [D23913105](https://our.internmc.facebook.com/intern/diff/D23913105)

[ghstack-poisoned]
if self.is_dynamic_quant:
# don't need to insert observer for output if activation does not
# need to be statically quantized
if not activation_is_statically_quantized(qconfig):
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use activation_is_dynamically_quantized instead here for better readability?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm planning to remove that check since it checks for both dynamic and weight only dtypes.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure I follow. What is the check we will use for dynamic quant in that case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

activation_is_dynamically_quantized checks for float32 dtype which can be both dynamic or weight only quantization.

@supriyar
Copy link
Contributor

Will we also be updating the top level APIs to remove the quantize_{static, dynamic}_fx calls ? https://github.com/pytorch/pytorch/blob/master/torch/quantization/quantize_fx.py#L108-L218

@jerryzh168
Copy link
Contributor Author

Will we also be updating the top level APIs to remove the quantize_{static, dynamic}_fx calls ? https://github.com/pytorch/pytorch/blob/master/torch/quantization/quantize_fx.py#L108-L218

yes will do that, I'm waiting for previous PRs to be landed to avoid conflicts.

Summary:
This PR merges all quantization mode and will only expose the following top level functions:
```
prepare_fx
prepare_qat_fx
convert_fx
```

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: [D23913105](https://our.internmc.facebook.com/intern/diff/D23913105)

[ghstack-poisoned]
Summary:
This PR merges all quantization mode and will only expose the following top level functions:
```
prepare_fx
prepare_qat_fx
convert_fx
```

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: [D23913105](https://our.internmc.facebook.com/intern/diff/D23913105)

[ghstack-poisoned]
Summary:
This PR merges all quantization mode and will only expose the following top level functions:
```
prepare_fx
prepare_qat_fx
convert_fx
```

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: [D23913105](https://our.internmc.facebook.com/intern/diff/D23913105)

[ghstack-poisoned]
jerryzh168 added a commit that referenced this pull request Sep 25, 2020
Summary:
This PR merges all quantization mode and will only expose the following top level functions:
```
prepare_fx
prepare_qat_fx
convert_fx
```

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

ghstack-source-id: 184888bba530fefefcf0fac48ead6b87c095c6a2
Pull Request resolved: #45292
Summary:
This PR merges all quantization mode and will only expose the following top level functions:
```
prepare_fx
prepare_qat_fx
convert_fx
```

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: [D23913105](https://our.internmc.facebook.com/intern/diff/D23913105)

[ghstack-poisoned]
Summary:
This PR merges all quantization mode and will only expose the following top level functions:
```
prepare_fx
prepare_qat_fx
convert_fx
```

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: [D23913105](https://our.internmc.facebook.com/intern/diff/D23913105)

[ghstack-poisoned]
jerryzh168 added a commit that referenced this pull request Sep 28, 2020
Summary:
This PR merges all quantization mode and will only expose the following top level functions:
```
prepare_fx
prepare_qat_fx
convert_fx
```

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

ghstack-source-id: 329ee8318e83e200afac2b77b855ee0627f2be20
Pull Request resolved: #45292
Summary:
This PR merges all quantization mode and will only expose the following top level functions:
```
prepare_fx
prepare_qat_fx
convert_fx
```

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: [D23913105](https://our.internmc.facebook.com/intern/diff/D23913105)

[ghstack-poisoned]
Summary:
This PR merges all quantization mode and will only expose the following top level functions:
```
prepare_fx
prepare_qat_fx
convert_fx
```

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: [D23913105](https://our.internmc.facebook.com/intern/diff/D23913105)

[ghstack-poisoned]
Summary:
This PR merges all quantization mode and will only expose the following top level functions:
```
prepare_fx
prepare_qat_fx
convert_fx
```

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: [D23913105](https://our.internmc.facebook.com/intern/diff/D23913105)

[ghstack-poisoned]
Summary:
This PR merges all quantization mode and will only expose the following top level functions:
```
prepare_fx
prepare_qat_fx
convert_fx
```

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: [D23913105](https://our.internmc.facebook.com/intern/diff/D23913105)

[ghstack-poisoned]
Summary:
This PR merges all quantization mode and will only expose the following top level functions:
```
prepare_fx
prepare_qat_fx
convert_fx
```

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: [D23913105](https://our.internmc.facebook.com/intern/diff/D23913105)

[ghstack-poisoned]
Summary:
This PR merges all quantization mode and will only expose the following top level functions:
```
prepare_fx
prepare_qat_fx
convert_fx
```

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: [D23913105](https://our.internmc.facebook.com/intern/diff/D23913105)

[ghstack-poisoned]
Summary:
This PR merges all quantization mode and will only expose the following top level functions:
```
prepare_fx
prepare_qat_fx
convert_fx
```

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: [D23913105](https://our.internmc.facebook.com/intern/diff/D23913105)

[ghstack-poisoned]
Summary:
This PR merges all quantization mode and will only expose the following top level functions:
```
prepare_fx
prepare_qat_fx
convert_fx
```

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: [D23913105](https://our.internmc.facebook.com/intern/diff/D23913105)

[ghstack-poisoned]
Summary:
This PR merges all quantization mode and will only expose the following top level functions:
```
prepare_fx
prepare_qat_fx
convert_fx
```

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: [D23913105](https://our.internmc.facebook.com/intern/diff/D23913105)

[ghstack-poisoned]
@facebook-github-bot
Copy link
Contributor

This pull request has been merged in ffcb098.

@t-vi
Copy link
Collaborator

t-vi commented Oct 1, 2020

This seems to break the CI on ROCm with an error that looks like it is this patch:
https://ci.pytorch.org/jenkins/job/pytorch-builds/job/pytorch-linux-bionic-rocm3.7-py3.6-test2/5653//console

FAIL: test_qat_prepare_device_affinity (quantization.test_quantize_fx.TestQuantizeFx)
06:50:29 ----------------------------------------------------------------------
06:50:29 Traceback (most recent call last):
06:50:29   File "/var/lib/jenkins/.local/lib/python3.6/site-packages/torch/testing/_internal/common_quantized.py", line 122, in test_fn
06:50:29     qfunction(*args, **kwargs)
06:50:29   File "/var/lib/jenkins/workspace/test/quantization/test_quantize_fx.py", line 268, in test_qat_prepare_device_affinity
06:50:29     model = prepare_fx(model, qconfig_dict)
06:50:29   File "/var/lib/jenkins/.local/lib/python3.6/site-packages/torch/quantization/quantize_fx.py", line 127, in prepare_fx
06:50:29     'eval mode'
06:50:29 AssertionError: prepare_fx only works for models ineval mode
06:50:29 

Also appreas to break pytorch_linux_xenial_cuda9_2_cudnn7_py3_gcc7_test with the same thing.

@mruberry
Copy link
Collaborator

mruberry commented Oct 1, 2020

Unlanding. @t-vi nailed it.

jerryzh168 added a commit that referenced this pull request Oct 1, 2020
Summary:
This PR merges all quantization mode and will only expose the following top level functions:
```
prepare_fx
prepare_qat_fx
convert_fx
```

Test Plan: Imported from OSS

Reviewed By: vkuzo

[ghstack-poisoned]
jerryzh168 added a commit that referenced this pull request Oct 1, 2020
…#45292)"

Summary:
This PR merges all quantization mode and will only expose the following top level functions:
```
prepare_fx
prepare_qat_fx
convert_fx
```

Test Plan: Imported from OSS

Reviewed By: vkuzo

Differential Revision: [D24053439](https://our.internmc.facebook.com/intern/diff/D24053439)

[ghstack-poisoned]
jerryzh168 added a commit that referenced this pull request Oct 1, 2020
Summary:
This PR merges all quantization mode and will only expose the following top level functions:
```
prepare_fx
prepare_qat_fx
convert_fx
```

Test Plan: Imported from OSS

Reviewed By: vkuzo

ghstack-source-id: 5208f72f119cea51fe4cf3fbf5358783415076d5
Pull Request resolved: #45672
facebook-github-bot pushed a commit that referenced this pull request Oct 1, 2020
…45672)

Summary:
Pull Request resolved: #45672

This PR merges all quantization mode and will only expose the following top level functions:
```
prepare_fx
prepare_qat_fx
convert_fx
```

Test Plan:
Imported from OSS

Imported from OSS

Reviewed By: z-a-f

Differential Revision: D24053439

fbshipit-source-id: 03d545e26a36bc22a73349061b751eeb35171e64
@facebook-github-bot facebook-github-bot deleted the gh/jerryzh168/440/head branch October 4, 2020 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants