[pytorch][PR] Add ability for a mobile::Module to save as flatbuffer #70201

qihqi · 2021-12-20T20:04:27Z

Summary:
Included functions:
save_mobile_module -> saves a mobile::Module to flatbuffer
load_mobile_module_from_file -> loads a flatbuffer into mobile::Module
parse_mobile_module -> parses from bytes or deserialized flatbuffer module object

Compared to previous attempts, this diff only adds flatbuffer to cmake target and leaves fbcode/xplat ones unchanged.

Test Plan: unittest

Differential Revision: D33239362

pytorch-probot · 2021-12-20T20:04:30Z

CI Flow Status

⚛️ CI Flow

Ruleset - Version: v1
Ruleset - File: https://github.com/qihqi/pytorch/blob/d4af813ebb9bf9015b8f6d9d177733ccb783d826/.github/generated-ciflow-ruleset.json
PR ciflow labels: ciflow/default,ciflow/all

Workflows	Labels (bold enabled)	Status
Triggered Workflows
caffe2-linux-xenial-py3.7-gcc5.4	`ciflow/all`, `ciflow/cpu`, `ciflow/linux`, `ciflow/trunk`	✅ triggered
docker-builds	`ciflow/all`, `ciflow/trunk`	✅ triggered
ios-12-5-1-arm64	`ciflow/all`, `ciflow/ios`, `ciflow/macos`, `ciflow/trunk`	✅ triggered
ios-12-5-1-arm64-coreml	`ciflow/all`, `ciflow/ios`, `ciflow/macos`, `ciflow/trunk`	✅ triggered
ios-12-5-1-arm64-custom-ops	`ciflow/all`, `ciflow/ios`, `ciflow/macos`, `ciflow/trunk`	✅ triggered
ios-12-5-1-arm64-full-jit	`ciflow/all`, `ciflow/ios`, `ciflow/macos`, `ciflow/trunk`	✅ triggered
ios-12-5-1-arm64-metal	`ciflow/all`, `ciflow/ios`, `ciflow/macos`, `ciflow/trunk`	✅ triggered
ios-12-5-1-x86-64	`ciflow/all`, `ciflow/ios`, `ciflow/macos`, `ciflow/trunk`	✅ triggered
ios-12-5-1-x86-64-coreml	`ciflow/all`, `ciflow/ios`, `ciflow/macos`, `ciflow/trunk`	✅ triggered
ios-12-5-1-x86-64-full-jit	`ciflow/all`, `ciflow/ios`, `ciflow/macos`, `ciflow/trunk`	✅ triggered
libtorch-linux-xenial-cuda10.2-py3.7-gcc7	`ciflow/all`, `ciflow/cuda`, `ciflow/libtorch`, `ciflow/linux`, `ciflow/trunk`	✅ triggered
libtorch-linux-xenial-cuda11.3-py3.7-gcc7	`ciflow/all`, `ciflow/cuda`, `ciflow/libtorch`, `ciflow/linux`, `ciflow/trunk`	✅ triggered
linux-bionic-cuda10.2-py3.9-gcc7	`ciflow/all`, `ciflow/cuda`, `ciflow/linux`, `ciflow/slow`, `ciflow/trunk`	✅ triggered
linux-bionic-py3.7-clang9	`ciflow/all`, `ciflow/cpu`, `ciflow/default`, `ciflow/linux`, `ciflow/noarch`, `ciflow/trunk`	✅ triggered
linux-docs	`ciflow/all`, `ciflow/cpu`, `ciflow/default`, `ciflow/docs`, `ciflow/linux`, `ciflow/trunk`	✅ triggered
linux-docs-push	`ciflow/all`, `ciflow/cpu`, `ciflow/linux`, `ciflow/scheduled`	✅ triggered
linux-vulkan-bionic-py3.7-clang9	`ciflow/all`, `ciflow/cpu`, `ciflow/default`, `ciflow/linux`, `ciflow/trunk`, `ciflow/vulkan`	✅ triggered
linux-xenial-cuda11.3-py3.7-gcc7	`ciflow/all`, `ciflow/cuda`, `ciflow/default`, `ciflow/linux`, `ciflow/trunk`	✅ triggered
linux-xenial-cuda11.3-py3.7-gcc7-bazel-test	`ciflow/all`, `ciflow/bazel`, `ciflow/cpu`, `ciflow/default`, `ciflow/linux`, `ciflow/trunk`	✅ triggered
linux-xenial-cuda11.3-py3.7-gcc7-no-ops	`ciflow/all`, `ciflow/cuda`, `ciflow/linux`, `ciflow/trunk`	✅ triggered
linux-xenial-py3-clang5-mobile-build	`ciflow/all`, `ciflow/default`, `ciflow/linux`, `ciflow/mobile`, `ciflow/trunk`	✅ triggered
linux-xenial-py3-clang5-mobile-custom-build-static	`ciflow/all`, `ciflow/default`, `ciflow/linux`, `ciflow/mobile`, `ciflow/trunk`	✅ triggered
linux-xenial-py3.7-clang7-asan	`ciflow/all`, `ciflow/cpu`, `ciflow/default`, `ciflow/linux`, `ciflow/sanitizers`, `ciflow/trunk`	✅ triggered
linux-xenial-py3.7-clang7-onnx	`ciflow/all`, `ciflow/cpu`, `ciflow/default`, `ciflow/linux`, `ciflow/onnx`, `ciflow/trunk`	✅ triggered
linux-xenial-py3.7-gcc5.4	`ciflow/all`, `ciflow/cpu`, `ciflow/default`, `ciflow/linux`, `ciflow/trunk`	✅ triggered
linux-xenial-py3.7-gcc7	`ciflow/all`, `ciflow/cpu`, `ciflow/default`, `ciflow/linux`, `ciflow/trunk`	✅ triggered
linux-xenial-py3.7-gcc7-no-ops	`ciflow/all`, `ciflow/cpu`, `ciflow/default`, `ciflow/linux`, `ciflow/trunk`	✅ triggered
macos-10-15-py3-arm64	`ciflow/all`, `ciflow/macos`, `ciflow/trunk`	✅ triggered
macos-10-15-py3-lite-interpreter-x86-64	`ciflow/all`, `ciflow/macos`, `ciflow/trunk`	✅ triggered
macos-11-py3-x86-64	`ciflow/all`, `ciflow/macos`, `ciflow/trunk`	✅ triggered
parallelnative-linux-xenial-py3.7-gcc5.4	`ciflow/all`, `ciflow/cpu`, `ciflow/linux`, `ciflow/trunk`	✅ triggered
periodic-libtorch-linux-bionic-cuda11.5-py3.7-gcc7	`ciflow/all`, `ciflow/cuda`, `ciflow/libtorch`, `ciflow/linux`, `ciflow/scheduled`	✅ triggered
periodic-libtorch-linux-xenial-cuda11.1-py3.7-gcc7	`ciflow/all`, `ciflow/cuda`, `ciflow/libtorch`, `ciflow/linux`, `ciflow/scheduled`	✅ triggered
periodic-linux-bionic-cuda11.5-py3.7-gcc7	`ciflow/all`, `ciflow/cuda`, `ciflow/linux`, `ciflow/scheduled`	✅ triggered
periodic-linux-xenial-cuda10.2-py3-gcc7-slow-gradcheck	`ciflow/all`, `ciflow/cuda`, `ciflow/linux`, `ciflow/scheduled`, `ciflow/slow`, `ciflow/slow-gradcheck`	✅ triggered
periodic-linux-xenial-cuda11.1-py3.7-gcc7-debug	`ciflow/all`, `ciflow/cuda`, `ciflow/linux`, `ciflow/scheduled`	✅ triggered
periodic-win-vs2019-cuda11.1-py3	`ciflow/all`, `ciflow/cuda`, `ciflow/scheduled`, `ciflow/win`	✅ triggered
periodic-win-vs2019-cuda11.5-py3	`ciflow/all`, `ciflow/cuda`, `ciflow/scheduled`, `ciflow/win`	✅ triggered
pytorch-linux-xenial-py3-clang5-android-ndk-r19c-build	`ciflow/all`, `ciflow/android`, `ciflow/cpu`, `ciflow/linux`, `ciflow/trunk`	✅ triggered
pytorch-linux-xenial-py3-clang5-android-ndk-r19c-gradle-custom-build-single	`ciflow/all`, `ciflow/android`, `ciflow/cpu`, `ciflow/default`, `ciflow/linux`, `ciflow/trunk`	✅ triggered
pytorch-linux-xenial-py3-clang5-android-ndk-r19c-gradle-custom-build-single-full-jit	`ciflow/all`, `ciflow/android`, `ciflow/cpu`, `ciflow/default`, `ciflow/linux`, `ciflow/trunk`	✅ triggered
win-vs2019-cpu-py3	`ciflow/all`, `ciflow/cpu`, `ciflow/default`, `ciflow/trunk`, `ciflow/win`	✅ triggered
win-vs2019-cuda11.3-py3	`ciflow/all`, `ciflow/cuda`, `ciflow/default`, `ciflow/trunk`, `ciflow/win`	✅ triggered
Skipped Workflows
linux-binary-conda	`ciflow/binaries`, `ciflow/binaries/conda`	🚫 skipped
linux-binary-libtorch-cxx11-abi	`ciflow/binaries`, `ciflow/binaries/libtorch`	🚫 skipped
linux-binary-libtorch-pre-cxx11	`ciflow/binaries`, `ciflow/binaries/libtorch`	🚫 skipped
linux-binary-manywheel	`ciflow/binaries`, `ciflow/binaries/wheel`	🚫 skipped

You can add a comment to the PR and tag @pytorchbot with the following commands:

# ciflow rerun, "ciflow/default" will always be added automatically
@pytorchbot ciflow rerun

# ciflow rerun with additional labels "-l <ciflow/label_name>", which is equivalent to adding these labels manually and trigger the rerun
@pytorchbot ciflow rerun -l ciflow/scheduled -l ciflow/slow

For more information, please take a look at the CI Flow Wiki.

facebook-github-bot · 2021-12-20T20:04:33Z

🔗 Helpful links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/70201
📄 Preview docs built from this PR
📄 Preview C++ docs built from this PR
🔧 Opt-in to CIFlow to control what jobs run on your PRs

💊 CI failures summary and remediations

As of commit d4af813 (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).

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

facebook-github-bot · 2021-12-20T20:04:53Z

This pull request was exported from Phabricator. Differential Revision: D33239362

facebook-github-bot · 2021-12-20T21:13:23Z

This pull request was exported from Phabricator. Differential Revision: D33239362

facebook-github-bot · 2021-12-20T21:48:45Z

This pull request was exported from Phabricator. Differential Revision: D33239362

qihqi · 2021-12-20T21:56:46Z

@pytorchbot ciflow rerun -l ciflow/all

facebook-github-bot · 2021-12-20T23:00:59Z

This pull request was exported from Phabricator. Differential Revision: D33239362

facebook-github-bot · 2021-12-21T00:09:42Z

This pull request was exported from Phabricator. Differential Revision: D33239362

facebook-github-bot · 2021-12-21T00:16:02Z

This pull request was exported from Phabricator. Differential Revision: D33239362

gmagogsfm · 2021-12-21T01:38:27Z

torch/csrc/jit/serialization/flatbuffer_serializer.cpp

Nit:
Create a constant for index of None, something like kNoneIndex

gmagogsfm · 2021-12-21T01:39:26Z

torch/csrc/jit/serialization/flatbuffer_serializer.cpp

What are causes of exceptions? Unable to hash an IValue?

Added more comments. IValue::hash explicitly throws runtime_error for some IValues. And for Tensor operator== doesn't return bool so you get c10::Error.

gmagogsfm · 2021-12-21T01:40:16Z

torch/csrc/jit/serialization/flatbuffer_serializer.cpp

Incomplete comment?

gmagogsfm · 2021-12-21T02:02:27Z

torch/csrc/jit/serialization/mobile_bytecode.fbs

What is this version supposed to be?

1 + whatever the latest (and last) pickle version should be. It is not explicitly used now.

gmagogsfm · 2021-12-21T02:06:26Z

torch/csrc/jit/serialization/flatbuffer_serializer.cpp

LOL, I think we are done with "pickling"

facebook-github-bot · 2021-12-21T02:41:23Z

This pull request was exported from Phabricator. Differential Revision: D33239362

facebook-github-bot · 2022-01-10T23:39:03Z

This pull request was exported from Phabricator. Differential Revision: D33239362

qihqi · 2022-01-11T17:45:22Z

Sync'd offline with @malfet and agreed to check-in the generated header file instead of messing with build scripts to generate it on the fly. PTAL.

gmagogsfm · 2022-01-11T17:54:19Z

Sync'd offline with @malfet and agreed to check-in the generated header file instead of messing with build scripts to generate it on the fly. PTAL.

LGTM

malfet · 2022-01-11T19:57:11Z

.jenkins/pytorch/win-build.sh

As headers are checked in, this should not longer be necessary, should it?

malfet · 2022-01-11T19:58:24Z

torch/CMakeLists.txt

LGTM, but in followup PR can we limit it to just 2 files in question? (So that we don't leak flatbuffers dependency to module extensions)

Do you mean we can use different include paths when compiling flatbuffer_loader.cpp and flatbuffer_serializer.cpp but not when compiling other .cpp files? How do I do that? Also it's not just those 2 files also ones that happens to include flatbuffer_loader.h or flatbuffer_serializer.h.

malfet · 2022-01-11T19:59:19Z

torch/CMakeLists.txt

Hmm, I though flatbuffers is header only ones its generated, isn't it?

malfet · 2022-01-11T20:01:09Z

Looks good to me, but please remove changes to windows-builds (they should no longer be necessary)
Also, can you please add to PR description projected binary size increase as result of this change?
And why can't it be an optional feature?

facebook-github-bot · 2022-01-11T20:16:00Z

This pull request was exported from Phabricator. Differential Revision: D33239362

facebook-github-bot · 2022-01-12T16:46:17Z

This pull request was exported from Phabricator. Differential Revision: D33239362

facebook-github-bot · 2022-01-12T17:03:38Z

This pull request was exported from Phabricator. Differential Revision: D33239362

facebook-github-bot · 2022-01-12T17:59:06Z

This pull request was exported from Phabricator. Differential Revision: D33239362

…ytorch#70201) Summary: Pull Request resolved: pytorch#70201 Included functions: save_mobile_module -> saves a mobile::Module to flatbuffer load_mobile_module_from_file -> loads a flatbuffer into mobile::Module parse_mobile_module -> parses from bytes or deserialized flatbuffer module object Compared to previous attempts, this diff only adds flatbuffer to cmake target and leaves fbcode/xplat ones unchanged. Test Plan: unittest Reviewed By: malfet, gmagogsfm Differential Revision: D33239362 fbshipit-source-id: c89d5f8b96fee6906a2df9f034c081865b34548f

facebook-github-bot · 2022-01-12T18:41:49Z

This pull request was exported from Phabricator. Differential Revision: D33239362

qihqi · 2022-01-12T21:57:39Z

Looks good to me, but please remove changes to windows-builds (they should no longer be necessary) Also, can you please add to PR description projected binary size increase as result of this change? And why can't it be an optional feature?

Update: removed change to win-build.sh and it seems to work. Binary size increase is ~15kb. We didn't propose as optional feature because this will replace Module._save_for_lite_interpreter().

qihqi · 2022-01-12T21:57:49Z

Looks good to me, but please remove changes to windows-builds (they should no longer be necessary) Also, can you please add to PR description projected binary size increase as result of this change? And why can't it be an optional feature?

Update: removed change to win-build.sh and it seems to work. Binary size increase is ~15kb. We didn't propose as optional feature because this will replace Module._save_for_lite_interpreter().

pytorch-probot bot added the ciflow/default label Dec 20, 2021

facebook-github-bot added the cla signed label Dec 20, 2021

facebook-github-bot added oncall: jit Add this issue/PR to JIT oncall triage queue fb-exported labels Dec 20, 2021

qihqi force-pushed the export-D33239362 branch from a628492 to f4ffa78 Compare December 20, 2021 21:13

qihqi requested a review from gmagogsfm December 20, 2021 21:15

qihqi force-pushed the export-D33239362 branch from f4ffa78 to 68e517b Compare December 20, 2021 21:48

pytorch-probot bot added the ciflow/all label Dec 20, 2021

pytorch-probot bot assigned pytorchbot and unassigned pytorchbot Dec 20, 2021

qihqi force-pushed the export-D33239362 branch from 68e517b to e68eb94 Compare December 20, 2021 23:00

qihqi force-pushed the export-D33239362 branch from e68eb94 to c39430f Compare December 21, 2021 00:09

qihqi force-pushed the export-D33239362 branch from c39430f to 089ada5 Compare December 21, 2021 00:16

gmagogsfm approved these changes Dec 21, 2021

View reviewed changes

qihqi requested review from janeyx99 and seemethere as code owners January 10, 2022 22:16

qihqi force-pushed the export-D33239362 branch from 6e0d98c to 61305a0 Compare January 10, 2022 23:39

gmagogsfm approved these changes Jan 11, 2022

View reviewed changes

malfet approved these changes Jan 11, 2022

View reviewed changes

qihqi force-pushed the export-D33239362 branch from 61305a0 to 6367571 Compare January 11, 2022 20:16

qihqi force-pushed the export-D33239362 branch from 6367571 to b23c8ce Compare January 12, 2022 16:46

qihqi force-pushed the export-D33239362 branch from b23c8ce to f911133 Compare January 12, 2022 17:03

qihqi force-pushed the export-D33239362 branch from f911133 to 3536506 Compare January 12, 2022 17:59

qihqi force-pushed the export-D33239362 branch from 3536506 to d4af813 Compare January 12, 2022 18:41

facebook-github-bot closed this in 1bc3571 Jan 13, 2022

[pytorch][PR] Add ability for a mobile::Module to save as flatbuffer #70201

[pytorch][PR] Add ability for a mobile::Module to save as flatbuffer #70201

Uh oh!

Conversation

qihqi commented Dec 20, 2021

Uh oh!

pytorch-probot bot commented Dec 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚛️ CI Flow

Uh oh!

facebook-github-bot commented Dec 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful links

💊 CI failures summary and remediations

Uh oh!

facebook-github-bot commented Dec 20, 2021

Uh oh!

facebook-github-bot commented Dec 20, 2021

Uh oh!

facebook-github-bot commented Dec 20, 2021

Uh oh!

qihqi commented Dec 20, 2021

Uh oh!

facebook-github-bot commented Dec 20, 2021

Uh oh!

facebook-github-bot commented Dec 21, 2021

Uh oh!

facebook-github-bot commented Dec 21, 2021

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!

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Dec 21, 2021

Uh oh!

facebook-github-bot commented Jan 10, 2022

Uh oh!

qihqi commented Jan 11, 2022

Uh oh!

gmagogsfm commented Jan 11, 2022

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!

malfet commented Jan 11, 2022

Uh oh!

facebook-github-bot commented Jan 11, 2022

Uh oh!

facebook-github-bot commented Jan 12, 2022

Uh oh!

facebook-github-bot commented Jan 12, 2022

Uh oh!

facebook-github-bot commented Jan 12, 2022

Uh oh!

facebook-github-bot commented Jan 12, 2022

Uh oh!

qihqi commented Jan 12, 2022

Uh oh!

qihqi commented Jan 12, 2022

pytorch-probot bot commented Dec 20, 2021 •

edited

Loading

facebook-github-bot commented Dec 20, 2021 •

edited

Loading