Skip to content

[pytorch] move legacy deserialization code into jit/import_legacy.cpp #25649

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

Closed
wants to merge 3 commits into from

Conversation

ljk53
Copy link
Contributor

@ljk53 ljk53 commented Sep 4, 2019

Stack from ghstack:

Summary:
Continue the work of PR #25493 to remove dependencies of generated
protobuf headers from jit/import.cpp.

Instead of adding intrusive #if/#else to gate the legacy functions,
moving them into a separate file. Keep the ScriptModuleDeserializer
structure as otherwise it will require a lot of interface changes.

There is not much state to copy from ScriptModuleDeserializer as it only
extracts extra_files before calling into LEGACY_deserialize. There is
no state to copy back into ScriptModuleDeserializer either as it directly
returns script::Module.

Test Plan:

  • builds;
  • with stacked PR to remove protobuf from cmake;
  • load and run ResNet-18 in model.json format with non-mobile build;
  • load and run ResNet-18 in pickle format with mobile build;

Differential Revision: D17183549

Summary:
Continue the work of PR #25493. Remove dependencies of generated
protobuf headers from jit/import.cpp.

Test Plan:
- builds;
- with stacked PR to remove protobuf from cmake;
…port.cpp"

Summary:
Continue the work of PR #25493. Remove dependencies of generated
protobuf headers from jit/import.cpp.

Test Plan:
- builds;
- with stacked PR to remove protobuf from cmake;

Differential Revision: [D17183549](https://our.internmc.facebook.com/intern/diff/D17183549)
@pytorchbot pytorchbot added caffe2 module: build Build system issues labels Sep 5, 2019
…port.cpp"

Summary:
Continue the work of PR #25493. Remove dependencies of generated
protobuf headers from jit/import.cpp.

Test Plan:
- builds;
- with stacked PR to remove protobuf from cmake;

Differential Revision: [D17183549](https://our.internmc.facebook.com/intern/diff/D17183549)
@ljk53 ljk53 changed the title [pytorch] strip out more protobuf dependencies from jit/import.cpp [pytorch] move legacy deserialization code into jit/import_legacy.cpp Sep 5, 2019
Copy link
Contributor

@zdevito zdevito left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks for the cleanup!

@facebook-github-bot
Copy link
Contributor

@ljk53 merged this pull request in a35a63b.

@facebook-github-bot facebook-github-bot deleted the gh/ljk53/26/head branch October 28, 2019 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
caffe2 Merged module: build Build system issues oncall: jit Add this issue/PR to JIT oncall triage queue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants