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

Port to new registration API (part 1) #36389

Closed
wants to merge 16 commits into from

Conversation

ezyang
Copy link
Contributor

@ezyang ezyang commented Apr 10, 2020

Stack from ghstack:

This is a roll up of a bunch of small PRs for ease of landing.

Differential Revision: D20964193

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

[ghstack-poisoned]
@dr-ci
Copy link

dr-ci bot commented Apr 10, 2020

💊 Build failures summary and remediations

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


💚 💚 Looks good so far! There are no failures yet. 💚 💚


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.

See how this bot performed.

This comment has been revised 74 times.

ezyang added 4 commits April 10, 2020 11:20
…Convolution"

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

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

[ghstack-poisoned]
…Convolution"

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

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

[ghstack-poisoned]
…Convolution"

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

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

[ghstack-poisoned]
…Convolution"

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

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

[ghstack-poisoned]
ezyang added 3 commits April 13, 2020 14:07
…Convolution"

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

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

[ghstack-poisoned]
…Convolution"

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

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

[ghstack-poisoned]
…Convolution"

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

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

[ghstack-poisoned]
…Convolution"

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

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

[ghstack-poisoned]
…Convolution"

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

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

[ghstack-poisoned]
ljk53 pushed a commit to ljk53/pytorch that referenced this pull request Apr 14, 2020
Signed-off-by: Edward Z. Yang <ezyang@fb.com>

ghstack-source-id: a99e00be9b56d6f8fd5555604236ebdd5195c4f7
Pull Request resolved: pytorch#36389
ezyang added 2 commits April 15, 2020 10:09
…Convolution"

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

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

[ghstack-poisoned]
…Convolution"

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

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

[ghstack-poisoned]
ezyang added 2 commits April 15, 2020 11:05
…Convolution"

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

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

[ghstack-poisoned]
…Convolution"

Signed-off-by: Edward Z. Yang <ezyang@fb.com>

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

[ghstack-poisoned]
@ezyang ezyang changed the title Update reference to RegisterOperators in error message in Convolution Port to new registration API (part 1) Apr 16, 2020
This is a roll up of a bunch of small PRs for ease of landing.

- Update reference to RegisterOperators in error message in Convolution. #36389
- Port Resize to use new registration API. #36390
- Port detach/detach to new registration API. #36512
- Add explicit schema for quantized conv/conv_prepack (fixes #36511). #36513
- Add a centralized TORCH_LIBRARY declaration for quantized and xnnpack ops (fixes #36510). #36520
- Functionalize qadd and register with new registration API. #36527
- Update quantized README for registering operators with new API. #36531
- Convert qbatch_norm to new operator registration API. #36534
- Convert qclamp to new operator registration API. #36535
- Functionalize qconcat and register with new registration API. #36536

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

[ghstack-poisoned]
ezyang added a commit that referenced this pull request Apr 16, 2020
This is a roll up of a bunch of small PRs for ease of landing.

- Update reference to RegisterOperators in error message in Convolution. #36389
- Port Resize to use new registration API. #36390
- Port detach/detach to new registration API. #36512
- Add explicit schema for quantized conv/conv_prepack (fixes #36511). #36513
- Add a centralized TORCH_LIBRARY declaration for quantized and xnnpack ops (fixes #36510). #36520
- Functionalize qadd and register with new registration API. #36527
- Update quantized README for registering operators with new API. #36531
- Convert qbatch_norm to new operator registration API. #36534
- Convert qclamp to new operator registration API. #36535
- Functionalize qconcat and register with new registration API. #36536

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

[ghstack-poisoned]
ezyang added a commit that referenced this pull request Apr 16, 2020
This is a roll up of a bunch of small PRs for ease of landing.

- Update reference to RegisterOperators in error message in Convolution. #36389
- Port Resize to use new registration API. #36390
- Port detach/detach to new registration API. #36512
- Add explicit schema for quantized conv/conv_prepack (fixes #36511). #36513
- Add a centralized TORCH_LIBRARY declaration for quantized and xnnpack ops (fixes #36510). #36520
- Functionalize qadd and register with new registration API. #36527
- Update quantized README for registering operators with new API. #36531
- Convert qbatch_norm to new operator registration API. #36534
- Convert qclamp to new operator registration API. #36535
- Functionalize qconcat and register with new registration API. #36536

ghstack-source-id: fe27bc1a60f82deee739116b557a2b31e11c3989
Pull Request resolved: #36389
This is a roll up of a bunch of small PRs for ease of landing.

- Update reference to RegisterOperators in error message in Convolution. #36389
- Port Resize to use new registration API. #36390
- Port detach/detach to new registration API. #36512
- Add explicit schema for quantized conv/conv_prepack (fixes #36511). #36513
- Add a centralized TORCH_LIBRARY declaration for quantized and xnnpack ops (fixes #36510). #36520
- Functionalize qadd and register with new registration API. #36527
- Update quantized README for registering operators with new API. #36531
- Convert qbatch_norm to new operator registration API. #36534
- Convert qclamp to new operator registration API. #36535
- Functionalize qconcat and register with new registration API. #36536

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

[ghstack-poisoned]
ezyang added a commit that referenced this pull request Apr 16, 2020
This is a roll up of a bunch of small PRs for ease of landing.

- Update reference to RegisterOperators in error message in Convolution. #36389
- Port Resize to use new registration API. #36390
- Port detach/detach to new registration API. #36512
- Add explicit schema for quantized conv/conv_prepack (fixes #36511). #36513
- Add a centralized TORCH_LIBRARY declaration for quantized and xnnpack ops (fixes #36510). #36520
- Functionalize qadd and register with new registration API. #36527
- Update quantized README for registering operators with new API. #36531
- Convert qbatch_norm to new operator registration API. #36534
- Convert qclamp to new operator registration API. #36535
- Functionalize qconcat and register with new registration API. #36536

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

[ghstack-poisoned]
@@ -34,6 +34,15 @@
('quantized::batch_norm', datetime.date(2020, 4, 20)),
('aten::sizes', datetime.date(2020, 4, 30)),
('aten::strides', datetime.date(2020, 4, 30)),
('quantized::conv_prepack', datetime.date(2020, 6, 1)),
Copy link
Collaborator

Choose a reason for hiding this comment

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

what's going on here? why do you need to put it into BC test?

Copy link
Contributor Author

@ezyang ezyang Apr 16, 2020

Choose a reason for hiding this comment

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

Oh, this is mistake from when I typoed conv_prepack for an registration, and that lit up the bc check, and then when I fixed the typo I didn't fix the registration

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 need this in the BC test because I made a (technical) BC breaking change.

Previously, conv_prepack was registered with inferred schema:

-        .op("quantized::conv_prepack", // conv_prepack is deprecated, please use
-                                       // conv2d_prepack for 2D conv.

With this PR, it is now done with explicit schema:

+  // conv_prepack is deprecated, please use conv2d_prepack for 2D conv.
+  m.def("conv_prepack(Tensor weight, Tensor? bias, int[] stride, int[] padding, int[] dilation, int groups) -> Tensor");

Moving from implicit to explicit is considered bc breaking by the bc script, thus I must whitelist it.

@kostmo
Copy link
Member

kostmo commented Apr 17, 2020

Appears to have caused the following in master:

Apr 17 00:15:27 ======================================================================
Apr 17 00:15:27 ERROR [0.374s]: test_batch_norm2d (__main__.TestQuantizedOps)
Apr 17 00:15:27 ----------------------------------------------------------------------
Apr 17 00:15:27 Traceback (most recent call last):
Apr 17 00:15:27   File "quantization/test_quantized.py", line 1534, in test_batch_norm2d
Apr 17 00:15:27     min_side=1, max_side=32),
Apr 17 00:15:27   File "/var/lib/jenkins/.local/lib/python3.7/site-packages/hypothesis/core.py", line 1116, in wrapped_test
Apr 17 00:15:27     raise the_error_hypothesis_found
Apr 17 00:15:27   File "quantization/test_quantized.py", line 1554, in test_batch_norm2d
Apr 17 00:15:27     qy = torch.ops.quantized.batch_norm2d(qx, weight, bias, mean, var, eps, Y_scale, Y_zero_point)
Apr 17 00:15:27   File "/opt/python/nightly/lib/python3.7/site-packages/torch/_ops.py", line 61, in __getattr__
Apr 17 00:15:27     op = torch._C._jit_get_operation(qualified_op_name)

Reverting.

@facebook-github-bot
Copy link
Contributor

@ezyang merged this pull request in 6615886.

@ezyang
Copy link
Contributor Author

ezyang commented Apr 17, 2020

Stunning, the local ci was all green! XD

@facebook-github-bot facebook-github-bot deleted the gh/ezyang/715/head branch April 20, 2020 14:17
okly366 pushed a commit to okly366/pytorch that referenced this pull request Apr 26, 2020
Signed-off-by: Edward Z. Yang <ezyang@fb.com>

ghstack-source-id: a99e00be9b56d6f8fd5555604236ebdd5195c4f7
Pull Request resolved: pytorch/pytorch#36389
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.

6 participants