[Lite Interpreter] Enable __setstate__ by iseeyuan · Pull Request #33294 · pytorch/pytorch

iseeyuan · 2020-02-13T16:19:15Z

Stack from ghstack:

[Lite Interpreter] Enable __setstate__ #33294 [Lite Interpreter] Enable setstate

Serialize bytecode of setstate and run it when loading the model.
One use case is quantization. To test this use case a few operators are registered temporarily for lite interpreter. The "_" prefix registration will be removed when the operators are all migrated to mobile.

Differential Revision: D20162898

dr-ci · 2020-02-13T16:28:05Z

💊 CircleCI build failures summary and remediations

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

Commit 7d2d498 was recently pushed. Waiting for builds...

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.

This comment has been revised 38 times.

ghstack-source-id: 351e4ec Pull Request resolved: #33294 [Lite Interpreter] Serialize/deserialize __setstate__ ghstack-source-id: 351e4ec Pull Request resolved: #33295

kimishpatel · 2020-02-28T17:46:41Z

+  }
+
+  // operators
+  std::vector<IValue> operators;


Given we know the vector size, we can just reserve that here?

kimishpatel · 2020-02-28T17:48:36Z

-};
+}
+
+static IValue Tup(std::vector<IValue> ivalues) {


Would it make sense to have a version of this that accepts rvalue ref? I see call-sites for this where passed in vector is not used afterwards and we still end up copying it during the call to this function.

I assume Tup() is a simple helper function. Currently the vector is not used later but not guaranteed. In addition, it's in the export process where I don't think the perf saving (if there is any) with rvalue ref would be noticeable. To keep it simple I'd prefer one API for the helper function.

dreiss · 2020-02-28T18:08:48Z

Can you import this to phabricator so we can see the moved code detection?

kimishpatel · 2020-02-28T18:34:13Z

  push(*stack, ss.str());
 }

+void to_dtype_kernal(const c10::OperatorHandle& op, Stack* stack) {


Typo, kernal vs kernel.

kimishpatel · 2020-02-28T18:36:26Z

+       norm_index > static_cast<int64_t>(tuple->elements().size())) {
+     throw std::out_of_range("Tuple list index out of range");
+   }
+   stack->emplace_back(tuple->elements()[norm_index]);


Is there a reason to not use pack variant here?

kimishpatel

Left some comments. BTW, for quantized models to work we will also have to add non dynamic versions of linear_run and linear_prepack etc. ops. It is orthogonal to this PR but just wanted to bring it up.

1. Serialize bytecode of __setstate__ and run it when loading the model. 2. One use case is quantization. To test this use case a few operators are registered temporarily for lite interpreter. The "_" prefix registration will be removed when the operators are all migrated to mobile. Differential Revision: [D20162898](https://our.internmc.facebook.com/intern/diff/D20162898)

ghstack-source-id: 646caa4 Pull Request resolved: #33294 [Lite Interpreter] Serialize/deserialize __setstate__ ghstack-source-id: 646caa4 Pull Request resolved: #33295

1. Serialize bytecode of __setstate__ and run it when loading the model. 2. One use case is quantization. To test this use case a few operators are registered temporarily for lite interpreter. The "_" prefix registration will be removed when the operators are all migrated to mobile. Differential Revision: [D20162898](https://our.internmc.facebook.com/intern/diff/D20162898)

kimishpatel

LGTM.

1. Serialize bytecode of __setstate__ and run it when loading the model. 2. One use case is quantization. To test this use case a few operators are registered temporarily for lite interpreter. The "_" prefix registration will be removed when the operators are all migrated to mobile. Differential Revision: [D20162898](https://our.internmc.facebook.com/intern/diff/D20162898)

ghstack-source-id: ae93780 Pull Request resolved: #33294 [Lite Interpreter] Serialize/deserialize __setstate__ ghstack-source-id: ae93780 Pull Request resolved: #33295

zdevito

Thanks -- see the inline comment about how to find all the setstate methods, I believe this will miss some.

zdevito · 2020-03-02T21:07:34Z

        auto userObj = pop(stack).toObject();
+        // Mobile only: since the number of slots is not known, resize the numAttributes
+        // before setSlot.
+        while (userObj->type()->numAttributes() <= inst.X) {


The combination of while, and v.type() is not correct. I think it would be best to just set all member types to AnyType rather than give them a wrong type.

There's exception when I use AnyType. Should I disable checkNoAny()?

I have changed it to NonType, which may be close to "neglecting type in loaded obj" as @zdevito suggested. I feel it's easier than introducing another #ifdef inside setslot(), where some codes may be hidden from unit test.

zdevito · 2020-03-02T21:08:45Z

  methods_.emplace_back(std::move(fn));
 }

+Function* CompilationUnit::find_method_by_qn(const c10::QualifiedName& qn) {


_by_qn feels redundant and they are Funcitons not methods, so maybe just find_function.

zdevito · 2020-03-02T21:12:08Z

-      elements.push_back(Tup({func.qualname().qualifiedName(), table}));
-    }
-    writeArchive("bytecode", Tup(std::move(elements)));
+    // top-level methods


This approach will not find all the setstate methods that need to be saved. For instance, if there is a custom class as an attribute of a Module, this will not save it. The correct approach is already implemented for standard serialization: it collects all the classes/functions that have been referenced and makes sure to save them. This code should reuse the result of that analysis to know what setstate methods to produce.

Good catch! I've changed it the similar way as in pickler, to check ivalue of the module and its slots. checkHasValidSetGetState() is reused.

1. Serialize bytecode of __setstate__ and run it when loading the model. 2. One use case is quantization. To test this use case a few operators are registered temporarily for lite interpreter. The "_" prefix registration will be removed when the operators are all migrated to mobile. Differential Revision: [D20162898](https://our.internmc.facebook.com/intern/diff/D20162898)

ghstack-source-id: 21a1969 Pull Request resolved: #33294 [Lite Interpreter] Serialize/deserialize __setstate__ ghstack-source-id: 21a1969 Pull Request resolved: #33295

1. Serialize bytecode of __setstate__ and run it when loading the model. 2. One use case is quantization. To test this use case a few operators are registered temporarily for lite interpreter. The "_" prefix registration will be removed when the operators are all migrated to mobile. Differential Revision: [D20162898](https://our.internmc.facebook.com/intern/diff/D20162898)

ghstack-source-id: 1b08abf Pull Request resolved: #33294 [Lite Interpreter] Serialize/deserialize __setstate__ ghstack-source-id: 1b08abf Pull Request resolved: #33295

1. Serialize bytecode of __setstate__ and run it when loading the model. 2. One use case is quantization. To test this use case a few operators are registered temporarily for lite interpreter. The "_" prefix registration will be removed when the operators are all migrated to mobile. Differential Revision: [D20162898](https://our.internmc.facebook.com/intern/diff/D20162898)

ghstack-source-id: a9e159b Pull Request resolved: #33294 [Lite Interpreter] Serialize/deserialize __setstate__ ghstack-source-id: a9e159b Pull Request resolved: #33295

facebook-github-bot · 2020-03-06T05:14:59Z

@iseeyuan merged this pull request in ccf4d69.

ghstack-source-id: b1e18a5 Pull Request resolved: pytorch/pytorch#33294

Summary: Pull Request resolved: pytorch#33294 1. Serialize bytecode of __setstate__ and run it when loading the model. 2. One use case is quantization. To test this use case a few operators are registered temporarily for lite interpreter. The "_" prefix registration will be removed when the operators are all migrated to mobile. Test Plan: Imported from OSS Differential Revision: D20162898 Pulled By: iseeyuan fbshipit-source-id: 7a3180807bf38fbce594d86993896861f12bb58c

register ops for quantized linear.

643c48c

iseeyuan requested a review from apaszke as a code owner February 13, 2020 16:19

iseeyuan mentioned this pull request Feb 13, 2020

[Lite Interpreter] Serialize/deserialize/run __setstate__ #33295

Closed

facebook-github-bot added the oncall: jit Add this issue/PR to JIT oncall triage queue label Feb 13, 2020

Martin Yuan added 3 commits February 25, 2020 12:01

Update on "register ops for quantized linear."

784b8da

Update on "register ops for quantized linear."

99b63ad

Update on "register ops for quantized linear."

9215cce

iseeyuan pushed a commit that referenced this pull request Feb 27, 2020

register ops for quantized linear.

7a7c1dd

ghstack-source-id: 351e4ec Pull Request resolved: #33294 [Lite Interpreter] Serialize/deserialize __setstate__ ghstack-source-id: 351e4ec Pull Request resolved: #33295

iseeyuan changed the title ~~register ops for quantized linear.~~ [Lite Interpreter] Enable __setstate__ Feb 27, 2020

iseeyuan requested review from dreiss, jamesr66a, kimishpatel, ljk53 and zdevito February 27, 2020 00:57

kimishpatel reviewed Feb 28, 2020

View reviewed changes

kimishpatel requested changes Feb 28, 2020

View reviewed changes

iseeyuan pushed a commit that referenced this pull request Feb 29, 2020

register ops for quantized linear.

4852126

ghstack-source-id: 646caa4 Pull Request resolved: #33294 [Lite Interpreter] Serialize/deserialize __setstate__ ghstack-source-id: 646caa4 Pull Request resolved: #33295

iseeyuan requested a review from kimishpatel February 29, 2020 00:56

kimishpatel approved these changes Mar 2, 2020

View reviewed changes

iseeyuan pushed a commit that referenced this pull request Mar 2, 2020

register ops for quantized linear.

b376752

ghstack-source-id: ae93780 Pull Request resolved: #33294 [Lite Interpreter] Serialize/deserialize __setstate__ ghstack-source-id: ae93780 Pull Request resolved: #33295

zdevito reviewed Mar 2, 2020

View reviewed changes

iseeyuan pushed a commit that referenced this pull request Mar 2, 2020

register ops for quantized linear.

e370643

ghstack-source-id: 21a1969 Pull Request resolved: #33294 [Lite Interpreter] Serialize/deserialize __setstate__ ghstack-source-id: 21a1969 Pull Request resolved: #33295

iseeyuan pushed a commit that referenced this pull request Mar 3, 2020

register ops for quantized linear.

1043b00

ghstack-source-id: 1b08abf Pull Request resolved: #33294 [Lite Interpreter] Serialize/deserialize __setstate__ ghstack-source-id: 1b08abf Pull Request resolved: #33295

iseeyuan requested a review from zdevito March 3, 2020 23:49

iseeyuan pushed a commit that referenced this pull request Mar 5, 2020

register ops for quantized linear.

246c4fe

ghstack-source-id: a9e159b Pull Request resolved: #33294 [Lite Interpreter] Serialize/deserialize __setstate__ ghstack-source-id: a9e159b Pull Request resolved: #33295

facebook-github-bot closed this in ccf4d69 Mar 5, 2020

facebook-github-bot added the merged label Mar 6, 2020

facebook-github-bot deleted the gh/iseeyuan/47/head branch March 9, 2020 14:16

ezyang mentioned this pull request Apr 13, 2020

Meaning of _quantized namespace isn't documented #36510

Closed

mruberry added the Merged label Oct 28, 2020

karansachdev-1012 pushed a commit to karansachdev-1012/pytorch that referenced this pull request Feb 17, 2026

register ops for quantized linear.

6296467

ghstack-source-id: b1e18a5 Pull Request resolved: pytorch/pytorch#33294

Conversation

iseeyuan commented Feb 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dr-ci Bot commented Feb 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💊 CircleCI build failures summary and remediations

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!

dreiss commented Feb 28, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kimishpatel left a comment

Choose a reason for hiding this comment

Uh oh!

kimishpatel left a comment

Choose a reason for hiding this comment

Uh oh!

zdevito 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!

facebook-github-bot commented Mar 6, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

iseeyuan commented Feb 13, 2020 •

edited

Loading

dr-ci Bot commented Feb 13, 2020 •

edited

Loading