[pt1][quant] Add the serialization support for FP16 LSTM by jianyuh · Pull Request #26378 · pytorch/pytorch

jianyuh · 2019-09-17T23:48:31Z

Stack from ghstack:

[pt1][quant] Add the serialization support for FP16 LSTM #26378 [WIP][pt1][quant] Add the serialization support for FP16 LSTM

We would like to add the serialization support for FP16 LSTM.

Differential Revision: D17391638

We would like to add the serialization support for FP16 LSTM. Differential Revision: [D17391638](https://our.internmc.facebook.com/intern/diff/D17391638/) [ghstack-poisoned]

We would like to add the serialization support for FP16 LSTM. Differential Revision: [D17391638](https://our.internmc.facebook.com/intern/diff/D17391638/) ghstack-source-id: 90265148 Pull Request resolved: #26378

jamesr66a

LGTM

jamesr66a · 2019-09-18T17:55:26Z

torch/nn/quantized/dynamic/modules/rnn.py

+        # however there is a JIT compilation error without it. This is just used to
+        # workaround that error.
+        if dtype == torch.qint8:
+            self._orig_weight_values = self._all_weight_values


Can we just assign an annotated empty list here?

self._orig_weight_values = torch.jit.annotate(List[torch.Tensor], [])

Not sure if it works but it's worth a shot. I think the values should be deduplicated in the pickler logic, but it's probably better not to have this unused stuff hanging around in memory as well

I tried before but it didn't work. JIT doesn't allow to return different data types for different branches. The error message is

> ... > Type mismatch: dynamic_vals is set to type List[Tuple[Tensor, Optional[Tensor]]] in the true branch and type List[Tensor] in the false branch: > at /data/users/jianyuhuang/fbsource/fbcode/buck-out/dev/gen/caffe2/test/quantization#binary,link-tree/torch/nn/quantized/dynamic/modules/rnn.py:183:8 > self.batch_first, > self.dropout, > self.bidirectional, > self._all_weight_names, > self.__overloads__, > self.training, > self.dtype, > ) > > if self.dtype == torch.qint8: > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~... <--- HERE > > dynamic_vals = torch.jit.annotate(List[Tuple[torch.Tensor, Optional[torch.Tensor]]], > []) > > ...

…STM" We would like to add the serialization support for FP16 LSTM. Differential Revision: [D17391638](https://our.internmc.facebook.com/intern/diff/D17391638/) [ghstack-poisoned]

Pull Request resolved: #26378 We would like to add the serialization support for FP16 LSTM. ghstack-source-id: 90479104 Differential Revision: [D17391638](https://our.internmc.facebook.com/intern/diff/D17391638/)

jamesr66a · 2019-09-21T00:45:49Z

@jianyuh any chance you could land this soon?

jamesr66a · 2019-09-21T00:49:17Z

Oh I guess it doesn't actually work:

Previous return statement returned a value of type Tuple[Tuple[str, int, int, int, bool, bool, float, bool, List[str], Dict[str, List[str]], bool, int, List[Tensor]], List[Tuple[Tensor, Optional[Tensor]]]] but this return statement returns a value of type Tuple[Tuple[str, int, int, int, bool, bool, float, bool, List[str], Dict[str, List[str]], bool, int, List[Tensor]], List[Tensor]]:

            for i in range(len(self._all_weight_names)):
                dynamic_vals.append(torch.ops.quantized.linear_unpack(self._all_weight_values[i]))

            return vals, dynamic_vals
        else:
            dynamic_vals_fp16 = torch.jit.annotate(List[torch.Tensor], [])
            for i in range(len(self._all_weight_names)):
                dynamic_vals_fp16.append(self._all_weight_values[i])
            return vals, dynamic_vals_fp16
            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ <--- HERE

jianyuh · 2019-09-21T06:22:02Z

@jianyuh any chance you could land this soon?

Sorry I just flied back to the Bay Area. The serialization support for FP16 LSTM doesn't work yet. The current PR broke the unit test. Need to check the possible solution for the different return value between FP16 and INT8.

jamesr66a · 2019-09-23T17:00:49Z

@jianyuh can you just switch back to the scheme you had before (duplicating weight values)?

jianyuh · 2019-09-23T17:24:10Z

@jianyuh can you just switch back to the scheme you had before (duplicating weight values)?

Sure! Will check out that version.

…STM" We would like to add the serialization support for FP16 LSTM. Differential Revision: [D17391638](https://our.internmc.facebook.com/intern/diff/D17391638/) [ghstack-poisoned]

Pull Request resolved: #26378 We would like to add the serialization support for FP16 LSTM. ghstack-source-id: 90650081 Differential Revision: [D17391638](https://our.internmc.facebook.com/intern/diff/D17391638/)

jianyuh · 2019-09-24T05:56:05Z

@jamesr66a : Please check my current code. It now passes the unit test. However, it has both the duplication of orig_weights_values as well as orig_bias_values.

Maybe we should only store the names instead of values for all_weights? That is, we use the same register_buffer and only use get_attr to fetch those buffers (e.g., https://github.com/pytorch/pytorch/blob/master/torch/jit/quantized.py#L326)? In that way, we avoid the additional orig_bias_values for FP16 path.

gottbrath · 2019-09-26T17:22:18Z

Jianyu, James, Dima -- Please discuss and decide if this is in scope for 1.3 or not.

gottbrath · 2019-09-26T18:28:05Z

Jianyu, James, Dima -- Please discuss and decide if this is in scope for 1.3 or not.

sounds like the conclusion (in discussion elsewhere) is that this isn't in scope for 1.3.

pytorchbot · 2022-04-12T02:36:24Z

Looks like this PR hasn't been updated in a while so we're going to go ahead and mark this as Stale.
Feel free to remove the Stale label if you feel this was a mistake.
Stale pull requests will automatically be closed 30 days after being marked Stale

[WIP][pt1][quant] Add the serialization support for FP16 LSTM

f77dde8

We would like to add the serialization support for FP16 LSTM. Differential Revision: [D17391638](https://our.internmc.facebook.com/intern/diff/D17391638/) [ghstack-poisoned]

jianyuh requested a review from apaszke as a code owner September 17, 2019 23:48

pytorchbot added the module: nn Related to torch.nn label Sep 17, 2019

jianyuh mentioned this pull request Sep 17, 2019

[pt1][quant] Add the FP16 weight support for LSTM in dynamic_quantize #25975

Closed

jamesr66a approved these changes Sep 18, 2019

View reviewed changes

Update on "[WIP][pt1][quant] Add the serialization support for FP16 L…

9c145e3

…STM" We would like to add the serialization support for FP16 LSTM. Differential Revision: [D17391638](https://our.internmc.facebook.com/intern/diff/D17391638/) [ghstack-poisoned]

Update on "[WIP][pt1][quant] Add the serialization support for FP16 L…

d132f0e

…STM" We would like to add the serialization support for FP16 LSTM. Differential Revision: [D17391638](https://our.internmc.facebook.com/intern/diff/D17391638/) [ghstack-poisoned]

jianyuh changed the title ~~[WIP][pt1][quant] Add the serialization support for FP16 LSTM~~ [pt1][quant] Add the serialization support for FP16 LSTM Sep 24, 2019

facebook-github-bot added the cla signed label Oct 30, 2020

pytorchbot added the Stale label Apr 12, 2022

github-actions bot closed this May 12, 2022

facebook-github-bot deleted the gh/jianyuh/28/head branch June 11, 2022 14:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[pt1][quant] Add the serialization support for FP16 LSTM#26378

[pt1][quant] Add the serialization support for FP16 LSTM#26378
jianyuh wants to merge 3 commits intogh/jianyuh/28/basefrom
gh/jianyuh/28/head

jianyuh commented Sep 17, 2019 •

edited

Loading

Uh oh!

jamesr66a left a comment

Uh oh!

jamesr66a Sep 18, 2019

Uh oh!

jianyuh Sep 18, 2019

Uh oh!

jamesr66a commented Sep 21, 2019

Uh oh!

jamesr66a commented Sep 21, 2019 •

edited

Loading

Uh oh!

jianyuh commented Sep 21, 2019

Uh oh!

jamesr66a commented Sep 23, 2019

Uh oh!

jianyuh commented Sep 23, 2019

Uh oh!

jianyuh commented Sep 24, 2019 •

edited

Loading

Uh oh!

gottbrath commented Sep 26, 2019

Uh oh!

gottbrath commented Sep 26, 2019

Uh oh!

pytorchbot commented Apr 12, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

jianyuh commented Sep 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jamesr66a left a comment

Choose a reason for hiding this comment

Uh oh!

jamesr66a Sep 18, 2019

Choose a reason for hiding this comment

Uh oh!

jianyuh Sep 18, 2019

Choose a reason for hiding this comment

Uh oh!

jamesr66a commented Sep 21, 2019

Uh oh!

jamesr66a commented Sep 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jianyuh commented Sep 21, 2019

Uh oh!

jamesr66a commented Sep 23, 2019

Uh oh!

jianyuh commented Sep 23, 2019

Uh oh!

jianyuh commented Sep 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gottbrath commented Sep 26, 2019

Uh oh!

gottbrath commented Sep 26, 2019

Uh oh!

pytorchbot commented Apr 12, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jianyuh commented Sep 17, 2019 •

edited

Loading

jamesr66a commented Sep 21, 2019 •

edited

Loading

jianyuh commented Sep 24, 2019 •

edited

Loading