quantized conv: add support for graph mode BC testing, and increase coverage #43524

vkuzo · 2020-08-24T22:19:48Z

Stack from ghstack:

ConvPackedParams: remove legacy format #43651 ConvPackedParams: remove legacy format
Adding a version serialization type to ConvPackedParam #43086 Adding a version serialization type to ConvPackedParam
quantized conv: add support for graph mode BC testing, and increase coverage #43524 quantized conv: add support for graph mode BC testing, and increase coverage

Summary:

adds support for testing BC on data format and numerics for graph mode
quantized modules
using the above, adds coverage for quantized conv2d on graph mode

Test Plan:

python test/test_quantization.py TestSerialization.test_conv2d_nobias
python test/test_quantization.py TestSerialization.test_conv2d_graph
python test/test_quantization.py TestSerialization.test_conv2d_nobias_graph

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: D23335222

Summary: Adding coverage for quantized conv without bias in the backwards compatibility tests Test Plan: ``` python test/test_quantization.py TestSerialization.test_conv2d_nobias ``` Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

test/quantization/test_backward_compatibility.py

vkuzo · 2020-08-25T20:12:24Z

test/quantization/test_backward_compatibility.py

+        module_id = self.__class__.__module__
+        munged_id = remove_prefix(self.id(), module_id + ".")
+        test_file = os.path.realpath(sys.modules[module_id].__file__)
+        base_name = os.path.join(os.path.dirname(test_file),
+                                 "serialized",
+                                 munged_id)
+
+        subname_output = ""
+        if subname:
+            base_name += "_" + subname
+            subname_output = " ({})".format(subname)
+
+        input_file = base_name + ".input.pt"
+        state_dict_file = base_name + ".state_dict.pt"
+        scripted_module_file = base_name + ".scripted.pt"
+        traced_module_file = base_name + ".traced.pt"
+        expected_file = base_name + ".expected.pt"


yeah, there is potential to reuse some part of this code, it didn't seem worth it at first glance as it would increase complexity and the benefit would be small as not all of this can be reused. I can take another look if you prefer.

I don't have a strong preference, I just feel in general we should not be copying code.

test/quantization/test_backward_compatibility.py

… increase coverage" Summary: 1. adds support for testing BC on data format and numerics for graph mode quantized modules 2. using the above, adds coverage for quantized conv2d on graph mode Test Plan: ``` python test/test_quantization.py TestSerialization.test_conv2d_nobias python test/test_quantization.py TestSerialization.test_conv2d_graph python test/test_quantization.py TestSerialization.test_conv2d_nobias_graph ``` Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

… increase coverage" Summary: 1. adds support for testing BC on data format and numerics for graph mode quantized modules 2. using the above, adds coverage for quantized conv2d on graph mode Test Plan: ``` python test/test_quantization.py TestSerialization.test_conv2d_nobias python test/test_quantization.py TestSerialization.test_conv2d_graph python test/test_quantization.py TestSerialization.test_conv2d_nobias_graph ``` Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D23335222](https://our.internmc.facebook.com/intern/diff/D23335222) [ghstack-poisoned]

dr-ci · 2020-08-26T17:38:47Z

💊 CI failures summary and remediations

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

1/1 failures possibly* introduced in this PR
- 1/1 non-CircleCI failure(s)

Extra GitHub checks: 1 failed

Failed: Codecov - codecov/project

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 12 times.

z-a-f

LGTM

raghuramank100 · 2020-08-26T22:00:44Z

test/quantization/test_backward_compatibility.py

+            torch.save(scripted_q(input_tensor), expected_file)
+
+        input_tensor = torch.load(input_file)
+        qmodule_scripted = torch.jit.load(scripted_module_file)


Should we also have a test to catch regressions in the size of the quantized module files? We have had one issue before where the quantized model was storing both fp32 and quantized weights.

I agree, that would be valuable as well. It would probably be good for that live somewhere else since there could be things which don't break BC but change model size, and vice versa. For this PR stack we are doing manual testing of file size, and it's a reduction.

raghuramank100 · 2020-08-26T22:01:24Z

test/quantization/test_backward_compatibility.py

+        qmodule_scripted = torch.jit.load(scripted_module_file)
+        qmodule_traced = torch.jit.load(traced_module_file)
+        expected = torch.load(expected_file)
+        self.assertEqual(qmodule_scripted(input_tensor), expected, atol=prec)


Is there any value in comparing graph with eager expected outputs?

There is definitely value in that, looks like that's already done in test_quantize_jit.py, so by those tests and the fact that both eager and graph are tested here for BC numerics, we can prove that eager vs graph condition is satisfied indirectly.

… increase coverage" Summary: 1. adds support for testing BC on data format and numerics for graph mode quantized modules 2. using the above, adds coverage for quantized conv2d on graph mode Test Plan: ``` python test/test_quantization.py TestSerialization.test_conv2d_nobias python test/test_quantization.py TestSerialization.test_conv2d_graph python test/test_quantization.py TestSerialization.test_conv2d_nobias_graph ``` Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D23335222](https://our.internmc.facebook.com/intern/diff/D23335222) [ghstack-poisoned]

codecov · 2020-08-28T18:58:59Z

Codecov Report

Merging #43524 into gh/vkuzo/131/base will decrease coverage by 0.00%.
The diff coverage is n/a.

@@                  Coverage Diff                  @@
##           gh/vkuzo/131/base   #43524      +/-   ##
=====================================================
- Coverage              69.34%   69.33%   -0.01%     
=====================================================
  Files                    378      378              
  Lines                  46698    46698              
=====================================================
- Hits                   32381    32380       -1     
- Misses                 14317    14318       +1

Impacted Files	Coverage Δ
torch/testing/_internal/expecttest.py	`77.55% <0.00%> (-1.03%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c177d25...b2e469d. Read the comment docs.

facebook-github-bot · 2020-08-29T00:17:59Z

This pull request has been merged in af4ecb3.

quantized conv: add additional bc test

02b3a7a

Summary: Adding coverage for quantized conv without bias in the backwards compatibility tests Test Plan: ``` python test/test_quantization.py TestSerialization.test_conv2d_nobias ``` Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

vkuzo mentioned this pull request Aug 24, 2020

Adding a version serialization type to ConvPackedParam #43086

Closed

vkuzo requested review from jerryzh168 and supriyar August 24, 2020 22:21

supriyar approved these changes Aug 24, 2020

View reviewed changes

vkuzo changed the title ~~quantized conv: add additional bc test~~ quantized conv: add support for graph mode BC testing, and increase coverage Aug 25, 2020

jerryzh168 reviewed Aug 25, 2020

View reviewed changes

test/quantization/test_backward_compatibility.py Show resolved Hide resolved

vkuzo commented Aug 25, 2020

View reviewed changes

test/quantization/test_backward_compatibility.py Show resolved Hide resolved

vkuzo added 2 commits August 25, 2020 14:10

yns88 changed the base branch from gh/vkuzo/131/base to master August 26, 2020 16:50

yns88 changed the base branch from master to gh/vkuzo/131/base August 26, 2020 16:56

vkuzo mentioned this pull request Aug 26, 2020

ConvPackedParams: remove legacy format #43651

Closed

z-a-f approved these changes Aug 26, 2020

View reviewed changes

raghuramank100 reviewed Aug 26, 2020

View reviewed changes

vkuzo added 3 commits August 27, 2020 12:37

facebook-github-bot closed this in af4ecb3 Aug 28, 2020

facebook-github-bot added the merged label Aug 29, 2020

facebook-github-bot deleted the gh/vkuzo/131/head branch September 1, 2020 14:17

mruberry added the Merged label Oct 28, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

quantized conv: add support for graph mode BC testing, and increase coverage #43524

quantized conv: add support for graph mode BC testing, and increase coverage #43524

Uh oh!

vkuzo commented Aug 24, 2020 •

edited

Loading

Uh oh!

Uh oh!

vkuzo Aug 25, 2020

Uh oh!

jerryzh168 Aug 25, 2020

Uh oh!

Uh oh!

dr-ci bot commented Aug 26, 2020 •

edited

Loading

Uh oh!

z-a-f left a comment

Uh oh!

raghuramank100 Aug 26, 2020

Uh oh!

vkuzo Aug 26, 2020

Uh oh!

raghuramank100 Aug 26, 2020

Uh oh!

vkuzo Aug 26, 2020

Uh oh!

codecov bot commented Aug 28, 2020 •

edited

Loading

Uh oh!

facebook-github-bot commented Aug 29, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

quantized conv: add support for graph mode BC testing, and increase coverage #43524

quantized conv: add support for graph mode BC testing, and increase coverage #43524

Uh oh!

Conversation

vkuzo commented Aug 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

vkuzo Aug 25, 2020

Choose a reason for hiding this comment

Uh oh!

jerryzh168 Aug 25, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dr-ci bot commented Aug 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💊 CI failures summary and remediations

Extra GitHub checks: 1 failed

Uh oh!

z-a-f left a comment

Choose a reason for hiding this comment

Uh oh!

raghuramank100 Aug 26, 2020

Choose a reason for hiding this comment

Uh oh!

vkuzo Aug 26, 2020

Choose a reason for hiding this comment

Uh oh!

raghuramank100 Aug 26, 2020

Choose a reason for hiding this comment

Uh oh!

vkuzo Aug 26, 2020

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Aug 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

facebook-github-bot commented Aug 29, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

vkuzo commented Aug 24, 2020 •

edited

Loading

dr-ci bot commented Aug 26, 2020 •

edited

Loading

codecov bot commented Aug 28, 2020 •

edited

Loading