ConvPackedParams: remove legacy format #43651

vkuzo · 2020-08-26T20:15:43Z

Stack from ghstack:

ConvPackedParams: remove legacy format #43651 ConvPackedParams: remove legacy format

Summary:

This is a forward compatibility follow-up to
#43086. We switch the
conv serialization to output the v2 format instead of the v1 format.

The plan is to land this 1 - 2 weeks after the base PR.

Test Plan:

python test/test_quantization.py TestSerialization.test_conv2d_graph_v2
python test/test_quantization.py TestSerialization.test_conv2d_nobias_graph_v2

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: D23355480

Summary: This is a forward compatibility follow-up to #43086. We switch the conv serialization to output the v2 format instead of the v1 format. The plan is to land this 1 - 2 weeks after the base PR. Test Plan: ``` python test/test_quantization.py TestSerialization.test_conv2d_graph_v2 python test/test_quantization.py TestSerialization.test_conv2d_nobias_graph_v2 ``` Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

z-a-f

So the legacy format will be removed completely?

vkuzo · 2020-08-26T20:44:33Z

So the legacy format will be removed completely?

this removes the code for models to save the legacy format. The code to read the legacy format would still be there for awhile.

Summary: This is a forward compatibility follow-up to #43086. We switch the conv serialization to output the v2 format instead of the v1 format. The plan is to land this 1 - 2 weeks after the base PR. Test Plan: ``` python test/test_quantization.py TestSerialization.test_conv2d_graph_v2 python test/test_quantization.py TestSerialization.test_conv2d_nobias_graph_v2 ``` Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D23355480](https://our.internmc.facebook.com/intern/diff/D23355480) [ghstack-poisoned]

Summary: This is a forward compatibility follow-up to #43086. We switch the conv serialization to output the v2 format instead of the v1 format. The plan is to land this 1 - 2 weeks after the base PR. Test Plan: ``` python test/test_quantization.py TestSerialization.test_conv2d_graph_v2 python test/test_quantization.py TestSerialization.test_conv2d_nobias_graph_v2 ``` Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: cfeeb22 Pull Request resolved: #43651

dr-ci · 2020-08-26T23:01:59Z

💊 CI failures summary and remediations

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

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

ci.pytorch.org: 1 failed

Failed: pr/pytorch-linux-bionic-rocm3.7-py3.6

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

raghuramank100 · 2020-08-26T23:17:45Z

Why do we need to land this later and not at the same time as the previous PR?

vkuzo · 2020-08-27T00:00:46Z

Why do we need to land this later and not at the same time as the previous PR?

pretty much to handle this case:

person A gets a dogfooding app on a mobile device with PyTorch revision A (dogfooding apps shipped once a week)
person B creates an ML model on a server with PyTorch revision B

if revision A does not know how to handle models created with revision B, the behavior is undefined. So, we first create revision A which knows how to read the new format but still uses the old format. We then wait N days (to allow most people to get on revision A), and then delete the old path.

Summary: This is a forward compatibility follow-up to #43086. We switch the conv serialization to output the v2 format instead of the v1 format. The plan is to land this 1 - 2 weeks after the base PR. Test Plan: ``` python test/test_quantization.py TestSerialization.test_conv2d_graph_v2 python test/test_quantization.py TestSerialization.test_conv2d_nobias_graph_v2 ``` Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D23355480](https://our.internmc.facebook.com/intern/diff/D23355480) [ghstack-poisoned]

Summary: This is a forward compatibility follow-up to #43086. We switch the conv serialization to output the v2 format instead of the v1 format. The plan is to land this 1 - 2 weeks after the base PR. Test Plan: ``` python test/test_quantization.py TestSerialization.test_conv2d_graph_v2 python test/test_quantization.py TestSerialization.test_conv2d_nobias_graph_v2 ``` Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: b9484b2 Pull Request resolved: #43651

codecov · 2020-08-27T23:20:57Z

Codecov Report

❗ No coverage uploaded for pull request base (gh/vkuzo/132/base@708e13c). Click here to learn what that means.
The diff coverage is n/a.

@@                 Coverage Diff                  @@
##             gh/vkuzo/132/base   #43651   +/-   ##
====================================================
  Coverage                     ?   67.99%           
====================================================
  Files                        ?      382           
  Lines                        ?    49380           
  Branches                     ?        0           
====================================================
  Hits                         ?    33578           
  Misses                       ?    15802           
  Partials                     ?        0

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 708e13c...6c00d92. Read the comment docs.

Summary: This is a forward compatibility follow-up to #43086. We switch the conv serialization to output the v2 format instead of the v1 format. The plan is to land this 1 - 2 weeks after the base PR. Test Plan: ``` python test/test_quantization.py TestSerialization.test_conv2d_graph_v2 python test/test_quantization.py TestSerialization.test_conv2d_nobias_graph_v2 ``` Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D23355480](https://our.internmc.facebook.com/intern/diff/D23355480) [ghstack-poisoned]

Summary: This is a forward compatibility follow-up to #43086. We switch the conv serialization to output the v2 format instead of the v1 format. The plan is to land this 1 - 2 weeks after the base PR. Test Plan: ``` python test/test_quantization.py TestSerialization.test_conv2d_graph_v2 python test/test_quantization.py TestSerialization.test_conv2d_nobias_graph_v2 ``` Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: c5bd1ff Pull Request resolved: #43651

Summary: This is a forward compatibility follow-up to #43086. We switch the conv serialization to output the v2 format instead of the v1 format. The plan is to land this 1 - 2 weeks after the base PR. Test Plan: ``` python test/test_quantization.py TestSerialization.test_conv2d_graph_v2 python test/test_quantization.py TestSerialization.test_conv2d_nobias_graph_v2 ``` Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D23355480](https://our.internmc.facebook.com/intern/diff/D23355480) [ghstack-poisoned]

Summary: This is a forward compatibility follow-up to #43086. We switch the conv serialization to output the v2 format instead of the v1 format. The plan is to land this 1 - 2 weeks after the base PR. Test Plan: ``` python test/test_quantization.py TestSerialization.test_conv2d_graph_v2 python test/test_quantization.py TestSerialization.test_conv2d_nobias_graph_v2 ``` Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 2534b3b Pull Request resolved: #43651

Summary: This is a forward compatibility follow-up to #43086. We switch the conv serialization to output the v2 format instead of the v1 format. The plan is to land this 1 - 2 weeks after the base PR. Test Plan: ``` python test/test_quantization.py TestSerialization.test_conv2d_graph_v2 python test/test_quantization.py TestSerialization.test_conv2d_nobias_graph_v2 ``` Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D23355480](https://our.internmc.facebook.com/intern/diff/D23355480) [ghstack-poisoned]

Summary: This is a forward compatibility follow-up to #43086. We switch the conv serialization to output the v2 format instead of the v1 format. The plan is to land this 1 - 2 weeks after the base PR. Test Plan: ``` python test/test_quantization.py TestSerialization.test_conv2d_graph_v2 python test/test_quantization.py TestSerialization.test_conv2d_nobias_graph_v2 ``` Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 864f62e Pull Request resolved: #43651

Summary: This is a forward compatibility follow-up to #43086. We switch the conv serialization to output the v2 format instead of the v1 format. The plan is to land this 1 - 2 weeks after the base PR. Test Plan: ``` python test/test_quantization.py TestSerialization.test_conv2d_graph_v2 python test/test_quantization.py TestSerialization.test_conv2d_nobias_graph_v2 ``` Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D23355480](https://our.internmc.facebook.com/intern/diff/D23355480) [ghstack-poisoned]

Summary: This is a forward compatibility follow-up to #43086. We switch the conv serialization to output the v2 format instead of the v1 format. The plan is to land this 1 - 2 weeks after the base PR. Test Plan: ``` python test/test_quantization.py TestSerialization.test_conv2d_graph_v2 python test/test_quantization.py TestSerialization.test_conv2d_nobias_graph_v2 ``` Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: e57f6c6 Pull Request resolved: #43651

Summary: This is a forward compatibility follow-up to #43086. We switch the conv serialization to output the v2 format instead of the v1 format. The plan is to land this 1 - 2 weeks after the base PR. Test Plan: ``` python test/test_quantization.py TestSerialization.test_conv2d_graph_v2 python test/test_quantization.py TestSerialization.test_conv2d_nobias_graph_v2 ``` Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D23355480](https://our.internmc.facebook.com/intern/diff/D23355480) [ghstack-poisoned]

Summary: This is a forward compatibility follow-up to #43086. We switch the conv serialization to output the v2 format instead of the v1 format. The plan is to land this 1 - 2 weeks after the base PR. Test Plan: ``` python test/test_quantization.py TestSerialization.test_conv2d_graph_v2 python test/test_quantization.py TestSerialization.test_conv2d_nobias_graph_v2 ``` Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 5cf3b89 Pull Request resolved: #43651

Summary: This is a forward compatibility follow-up to #43086. We switch the conv serialization to output the v2 format instead of the v1 format. The plan is to land this 1 - 2 weeks after the base PR. Test Plan: ``` python test/test_quantization.py TestSerialization.test_conv2d_graph_v2 python test/test_quantization.py TestSerialization.test_conv2d_nobias_graph_v2 ``` Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D23355480](https://our.internmc.facebook.com/intern/diff/D23355480) [ghstack-poisoned]

Summary: This is a forward compatibility follow-up to #43086. We switch the conv serialization to output the v2 format instead of the v1 format. The plan is to land this 1 - 2 weeks after the base PR. Test Plan: ``` python test/test_quantization.py TestSerialization.test_conv2d_graph_v2 python test/test_quantization.py TestSerialization.test_conv2d_nobias_graph_v2 ``` Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 181a12c Pull Request resolved: #43651

vkuzo · 2020-09-09T20:30:27Z

aten/src/ATen/native/quantized/cpu/conv_serialization.h

@@ -167,9 +152,6 @@ ConvParamsSerializationType serialize_conv(
  auto output_padding = params->output_padding().vec();
  params_vec.insert(params_vec.end(), output_padding.begin(),
                    output_padding.end());
-  for (int i = 0; i < kSpatialDim; i++) {


Seems like this was left in #39714 by accident, and tests did not catch it. This PR adds test coverage for this path, so we need to fix it to land.

Summary: This is a forward compatibility follow-up to #43086. We switch the conv serialization to output the v2 format instead of the v1 format. The plan is to land this 1 - 2 weeks after the base PR. Test Plan: ``` python test/test_quantization.py TestSerialization.test_conv2d_graph_v2 python test/test_quantization.py TestSerialization.test_conv2d_nobias_graph_v2 ``` Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D23355480](https://our.internmc.facebook.com/intern/diff/D23355480) [ghstack-poisoned]

Summary: This is a forward compatibility follow-up to #43086. We switch the conv serialization to output the v2 format instead of the v1 format. The plan is to land this 1 - 2 weeks after the base PR. Test Plan: ``` python test/test_quantization.py TestSerialization.test_conv2d_graph_v2 python test/test_quantization.py TestSerialization.test_conv2d_nobias_graph_v2 ``` Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 297cfe0 Pull Request resolved: #43651

facebook-github-bot · 2020-09-10T22:16:38Z

This pull request has been merged in b3f0297.

This was referenced Aug 26, 2020

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

Closed

Adding a version serialization type to ConvPackedParam #43086

Closed

vkuzo linked an issue Aug 26, 2020 that may be closed by this pull request

placeholder to remove legacy conv packed params #43649

Closed

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

View reviewed changes

raghuramank100 self-requested a review August 26, 2020 23:17

raghuramank100 approved these changes Aug 26, 2020

View reviewed changes

vkuzo commented Sep 9, 2020

View reviewed changes

facebook-github-bot closed this in b3f0297 Sep 10, 2020

facebook-github-bot added the merged label Sep 10, 2020

facebook-github-bot deleted the gh/vkuzo/132/head branch September 14, 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!

ConvPackedParams: remove legacy format #43651

ConvPackedParams: remove legacy format #43651

Uh oh!

vkuzo commented Aug 26, 2020 •

edited

Loading

Uh oh!

z-a-f left a comment

Uh oh!

vkuzo commented Aug 26, 2020

Uh oh!

dr-ci bot commented Aug 26, 2020 •

edited

Loading

Uh oh!

raghuramank100 commented Aug 26, 2020

Uh oh!

vkuzo commented Aug 27, 2020

Uh oh!

codecov bot commented Aug 27, 2020 •

edited

Loading

Uh oh!

vkuzo Sep 9, 2020

Uh oh!

facebook-github-bot commented Sep 10, 2020

Uh oh!

Uh oh!

ConvPackedParams: remove legacy format #43651

ConvPackedParams: remove legacy format #43651

Uh oh!

Conversation

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

Uh oh!

Uh oh!

z-a-f left a comment

Choose a reason for hiding this comment

Uh oh!

vkuzo commented Aug 26, 2020

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

ci.pytorch.org: 1 failed

Uh oh!

raghuramank100 commented Aug 26, 2020

Uh oh!

vkuzo commented Aug 27, 2020

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

vkuzo Sep 9, 2020

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Sep 10, 2020

Uh oh!

Uh oh!

vkuzo commented Aug 26, 2020 •

edited

Loading

dr-ci bot commented Aug 26, 2020 •

edited

Loading

codecov bot commented Aug 27, 2020 •

edited

Loading