Refactor LSTM tests #38851

raghuramank100 · 2020-05-21T02:18:28Z

Stack from ghstack:

Enable dynamic quantized GRU support #39920 Enable dynamic quantized GRU support
Dynamic quantization support for GRU, LSTMCell, RNNCell and GRUCell #37159 Dynamic quantization support for GRU, LSTMCell, RNNCell and GRUCell
Refactor LSTM tests #38851 Refactor LSTM tests
Current LSTM tests do not check for scriptability. We also do not test that the module is consistent with the LSTM op. Modified the tests and refactored them to fit into the new testing structure
Differential Revision: D21628596

Differential Revision: [D21628596](https://our.internmc.facebook.com/intern/diff/D21628596/) [ghstack-poisoned]

Differential Revision: [D21628596](https://our.internmc.facebook.com/intern/diff/D21628596/) ghstack-source-id: 104494648 Pull Request resolved: #38851

dr-ci · 2020-05-21T02:30:13Z

💊 CI failures summary and remediations

As of commit 7f159ae (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-xenial-rocm3.3-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 129 times.

Differential Revision: [D21628596](https://our.internmc.facebook.com/intern/diff/D21628596/) [ghstack-poisoned]

Pull Request resolved: #38851 ghstack-source-id: 104570469 Differential Revision: [D21628596](https://our.internmc.facebook.com/intern/diff/D21628596/)

supriyar · 2020-05-27T20:55:54Z

test/quantization/test_quantized_module.py

+        # Check that module matches the numerics of the op and ensure that module can be
+        # instantiated for all engines and dtypes
+
+        for qengine in supported_qengines:


You can use the @override_qengines decorator instead of these two lines.

supriyar

Should we also add a test at op level for quantized lstm? That tests for different input shapes and layers? I don't see one in test_quantized_op.py

test/quantization/test_quantize.py

Current LSTM tests do not check for scriptability. We also do not test that the module is consistent with the LSTM op. Modified the tests and refactored them to fit into the new testing structure Differential Revision: [D21628596](https://our.internmc.facebook.com/intern/diff/D21628596/) [ghstack-poisoned]

Pull Request resolved: #38851 ghstack-source-id: 104924757 Differential Revision: [D21628596](https://our.internmc.facebook.com/intern/diff/D21628596/)

Current LSTM tests do not check for scriptability. We also do not test that the module is consistent with the LSTM op. Modified the tests and refactored them to fit into the new testing structure Differential Revision: [D21628596](https://our.internmc.facebook.com/intern/diff/D21628596/) [ghstack-poisoned]

Pull Request resolved: #38851 ghstack-source-id: 105587461 Differential Revision: [D21628596](https://our.internmc.facebook.com/intern/diff/D21628596/)

Current LSTM tests do not check for scriptability. We also do not test that the module is consistent with the LSTM op. Modified the tests and refactored them to fit into the new testing structure Differential Revision: [D21628596](https://our.internmc.facebook.com/intern/diff/D21628596/) [ghstack-poisoned]

Create three tests for LSTMs: 1. test_qlstm: Test to check numerics of quantized LSTM operator. 2. test_lstm_api: To check the LSTM module and compare it with the quantized LSTM op 3. test_quantized_rnn: Check the dynamic quantization workflow, scriptability and serialization of quantized LSTM Pull Request resolved: #38851 ghstack-source-id: 105864340 Differential Revision: [D21628596](https://our.internmc.facebook.com/intern/diff/D21628596/)

Current LSTM tests do not check for scriptability. We also do not test that the module is consistent with the LSTM op. Modified the tests and refactored them to fit into the new testing structure Differential Revision: [D21628596](https://our.internmc.facebook.com/intern/diff/D21628596/) [ghstack-poisoned]

z-a-f

lgtm

supriyar · 2020-06-15T18:22:59Z

test/quantization/test_quantized_op.py

+                if dtype == torch.qint8:
+                    packed_ih = torch.ops.quantized.linear_prepack(Wq1, b1)
+                    packed_hh = torch.ops.quantized.linear_prepack(Wq2, b2)
+                    cell_params = torch.ops.quantized.make_quantized_cell_params_dynamic(packed_ih, packed_hh, b1, b2)


We should set reduce_range True here.

supriyar · 2020-06-15T18:26:12Z

test/quantization/test_quantize.py

-
-        ref_out, ref_hid = ref(x, hiddens)
-
        for qengine in supported_qengines:


Can we use @override_qengines decorator here?

supriyar · 2020-06-15T18:26:53Z

test/quantization/test_quantize.py


    def test_quantized_rnn(self):
-        r"""Test execution and serialization for dynamic quantized lstm modules on int8 and fp16
+        r"""Test dynamic quantization, scriptability and serialization for dynamic quantized lstm modules on int8 and fp16


There is a test below test_per_channel_lstm_quantize for per-channel quantization. Should we combine the two here and go over both qconfigs?

Yes, I will combine the tests

supriyar · 2020-06-15T18:27:16Z

test/quantization/test_quantized_op.py

+        def _get_rnn_inputs(seq_len, num_batches, input_size, hidden_size, num_directions):
+            # For Input (seq_len, batch, input_size)
+            X = torch.randn(seq_len, num_batches, input_size)
+            # TODO: Change to reduce_range=True once support is enabled


This is supported now

I have it enabled in the next PR in the stack, will move the change to this PR itself.

supriyar · 2020-06-15T18:27:37Z

test/quantization/test_quantized_op.py

+                H = torch.zeros(num_directions, num_batches, hidden_size)
+                C = torch.zeros(num_directions, num_batches, hidden_size)
+
+            s, z = _calculate_dynamic_qparams(H, torch.quint8, reduce_range=False)


Change reduce_range to true here

supriyar · 2020-06-15T18:27:43Z

test/quantization/test_quantized_op.py

+
+            s, z = _calculate_dynamic_qparams(H, torch.quint8, reduce_range=False)
+            Hq = torch.quantize_per_tensor(H, s, z, torch.quint8)
+            s, z = _calculate_dynamic_qparams(C, torch.quint8, reduce_range=False)


Thanks, have made all the changes requested.

supriyar

Left a few comments that I think should be addressed before submitting.

z-a-f

Testing the differential approval

z-a-f

Testing phabricator

z-a-f

Testing the phabricator

Current LSTM tests do not check for scriptability. We also do not test that the module is consistent with the LSTM op. Modified the tests and refactored them to fit into the new testing structure Differential Revision: [D21628596](https://our.internmc.facebook.com/intern/diff/D21628596/) [ghstack-poisoned]

z-a-f

Testing phabricator

Current LSTM tests do not check for scriptability. We also do not test that the module is consistent with the LSTM op. Modified the tests and refactored them to fit into the new testing structure Differential Revision: [D21628596](https://our.internmc.facebook.com/intern/diff/D21628596/) [ghstack-poisoned]

facebook-github-bot · 2020-06-16T10:10:12Z

This pull request has been merged in 655f1ea.

Summary: Create three tests for LSTMs: 1. test_qlstm: Test to check numerics of quantized LSTM operator. 2. test_lstm_api: To check the LSTM module and compare it with the quantized LSTM op 3. test_quantized_rnn: Check the dynamic quantization workflow, scriptability and serialization of quantized LSTM Pull Request resolved: pytorch#38851 ghstack-source-id: 105945574 (Note: this ignores all push blocking failures!) Test Plan: buck test caffe2/test:quantization -- 'test_lstm_api \(quantization\.test_quantized_module\.TestDynamicQuantizedModule\)' --print-passing-details buck test caffe2/test:quantization -- 'test_quantized_rnn \(quantization\.test_quantize\.TestPostTrainingDynamic\)' buck test caffe2/test:quantization -- 'test_qlstm \(quantization\.test_quantized_op\.TestDynamicQuantizedRNNOp\)' --print-passing-details Differential Revision: D21628596 fbshipit-source-id: 4aeda899f2e5f14bfbe3d82096cb4ce89c725fa1

Refactor LSTM tests

3c08a61

Differential Revision: [D21628596](https://our.internmc.facebook.com/intern/diff/D21628596/) [ghstack-poisoned]

raghuramank100 pushed a commit that referenced this pull request May 21, 2020

Refactor LSTM tests

5bede35

Differential Revision: [D21628596](https://our.internmc.facebook.com/intern/diff/D21628596/) ghstack-source-id: 104494648 Pull Request resolved: #38851

Update on "Refactor LSTM tests"

df7c394

Differential Revision: [D21628596](https://our.internmc.facebook.com/intern/diff/D21628596/) [ghstack-poisoned]

Update on "Refactor LSTM tests"

bba120d

Differential Revision: [D21628596](https://our.internmc.facebook.com/intern/diff/D21628596/) [ghstack-poisoned]

Update on "Refactor LSTM tests"

659afae

Differential Revision: [D21628596](https://our.internmc.facebook.com/intern/diff/D21628596/) [ghstack-poisoned]

Update on "Refactor LSTM tests"

ada5e05

Differential Revision: [D21628596](https://our.internmc.facebook.com/intern/diff/D21628596/) [ghstack-poisoned]

raghuramank100 pushed a commit that referenced this pull request May 22, 2020

Refactor LSTM tests

d24da7a

Pull Request resolved: #38851 ghstack-source-id: 104570469 Differential Revision: [D21628596](https://our.internmc.facebook.com/intern/diff/D21628596/)

raghuramank100 requested review from supriyar and jerryzh168 May 22, 2020 01:54

supriyar reviewed May 27, 2020

View reviewed changes

test/quantization/test_quantize.py Show resolved Hide resolved

raghuramank100 pushed a commit that referenced this pull request May 29, 2020

Refactor LSTM tests

7e83936

Pull Request resolved: #38851 ghstack-source-id: 104924757 Differential Revision: [D21628596](https://our.internmc.facebook.com/intern/diff/D21628596/)

raghuramank100 mentioned this pull request May 29, 2020

Dynamic quantization support for GRU, LSTMCell, RNNCell and GRUCell #37159

Closed

raghuramank10000 added 3 commits June 2, 2020 18:11

raghuramank100 pushed a commit that referenced this pull request Jun 10, 2020

Refactor LSTM tests

f098b72

Pull Request resolved: #38851 ghstack-source-id: 105587461 Differential Revision: [D21628596](https://our.internmc.facebook.com/intern/diff/D21628596/)

raghuramank10000 added 3 commits June 9, 2020 23:14

raghuramank100 mentioned this pull request Jun 12, 2020

Enable dynamic quantized GRU support #39920

Closed

raghuramank10000 added 3 commits June 12, 2020 00:10

raghuramank100 requested a review from apaszke as a code owner June 15, 2020 01:43

z-a-f approved these changes Jun 15, 2020

View reviewed changes

supriyar reviewed Jun 15, 2020

View reviewed changes

supriyar requested changes Jun 15, 2020

View reviewed changes

z-a-f approved these changes Jun 16, 2020

View reviewed changes

supriyar approved these changes Jun 16, 2020

View reviewed changes

raghuramank10000 added 3 commits June 15, 2020 19:21

facebook-github-bot closed this in 655f1ea Jun 16, 2020

facebook-github-bot added the merged label Jun 16, 2020

mruberry added the Merged label Oct 28, 2020


		ref_out, ref_hid = ref(x, hiddens)

		for qengine in supported_qengines:

Refactor LSTM tests #38851

Refactor LSTM tests #38851

Uh oh!

Conversation

raghuramank100 commented May 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dr-ci bot commented May 21, 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!

Choose a reason for hiding this comment

Uh oh!

supriyar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

z-a-f left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

supriyar left a comment

Choose a reason for hiding this comment

Uh oh!

z-a-f left a comment

Choose a reason for hiding this comment

Uh oh!

z-a-f left a comment

Choose a reason for hiding this comment

Uh oh!

z-a-f left a comment

Choose a reason for hiding this comment

Uh oh!

z-a-f left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Jun 16, 2020

Uh oh!

Uh oh!

raghuramank100 commented May 21, 2020 •

edited

Loading

dr-ci bot commented May 21, 2020 •

edited

Loading