Skip to content
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

PyTorchShim: Add serde callbacks to facilitate lazy loading models #796

Merged
merged 11 commits into from
Dec 8, 2022

Conversation

shadeMe
Copy link
Collaborator

@shadeMe shadeMe commented Oct 24, 2022

When attempting to lazily initialize/load PyTorch models, special care needs to be taken when deserializing trained models from disk. Since the init method of Models are not called during deserialization, shape inference cannot be performed before restoring model parameters. Therefore, the necessary information required to initialize the model must additionally be serialized to disk for later use during deserialization.

When attempting to lazily initialize/load PyTorch models, special care needs to be taken when deserializing trained models from disk. Since the `init` method of `Model`s are not called during deserialization, shape inference  cannot be performed before restoring model parameters. Therefore, the necessary information required to initialize the model must additionaly be serialized to disk for later use during deserialization.
@shadeMe shadeMe added feat / shims Shims for PyTorch, TensorFlow etc. serialization Saving and loading models labels Oct 24, 2022
@shadeMe shadeMe requested a review from danieldk October 24, 2022 14:36
thinc/shims/pytorch.py Outdated Show resolved Hide resolved
@shadeMe shadeMe requested a review from danieldk October 25, 2022 09:30
Copy link
Contributor

@danieldk danieldk 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, but since this is a fairly large API extension, someone else should also look at it.

@adrianeboyd adrianeboyd removed their request for review October 25, 2022 14:46
@rmitsch rmitsch self-requested a review November 16, 2022 10:18
Copy link
Contributor

@rmitsch rmitsch left a comment

Choose a reason for hiding this comment

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

Looks reasonable from what I can tell with my limited exposure to thinc. Only minor comments.

thinc/layers/pytorchwrapper.py Outdated Show resolved Hide resolved
thinc/layers/pytorchwrapper.py Outdated Show resolved Hide resolved
thinc/shims/pytorch.py Show resolved Hide resolved
thinc/shims/pytorch.py Outdated Show resolved Hide resolved
thinc/shims/pytorch.py Show resolved Hide resolved
thinc/shims/pytorch.py Show resolved Hide resolved
@shadeMe shadeMe requested a review from rmitsch December 2, 2022 13:55
Copy link
Contributor

@rmitsch rmitsch left a comment

Choose a reason for hiding this comment

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

One minor comment, otherwise LGTM. Approving this to not block things unnecessarily.

thinc/layers/pytorchwrapper.py Show resolved Hide resolved
shadeMe and others added 2 commits December 8, 2022 11:12
thinc/layers/pytorchwrapper.py Outdated Show resolved Hide resolved
thinc/layers/pytorchwrapper.py Outdated Show resolved Hide resolved
thinc/layers/pytorchwrapper.py Outdated Show resolved Hide resolved
thinc/tests/layers/test_pytorch_wrapper.py Outdated Show resolved Hide resolved
@danieldk danieldk merged commit 2f45b6d into explosion:master Dec 8, 2022
@shadeMe shadeMe deleted the shim-serialization-callbacks branch December 9, 2022 01:42
@danieldk danieldk mentioned this pull request Dec 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat / shims Shims for PyTorch, TensorFlow etc. serialization Saving and loading models
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants