[jit] Add type tags to lists/dicts in pickle#33255
Conversation
|
Thanks for putting this PR quickly! If the types are embedded in pickle, we may not need restoreAccurateTypeTags here: pytorch/torch/csrc/jit/import.cpp Line 160 in d554b11 |
|
LGTM. Thanks! Would be great if @zdevito could take a look before it's landed. |
zdevito
left a comment
There was a problem hiding this comment.
This looks pretty good, but we have some file format versioning to fix before it can land. We need to bump kProducedFileFormatVersion, because now there will be restore_type_tags in the IR. While we are doing this, we also need to remove the list specialization functions (build_intlist, etc.) from serialization. They are no longer needed and the only reason I haven't removed them is because of breaking forward compat. Now that we are doing it anyway, we should do both at once. Also, restoreAccurateTypeTags should only be called for the older format since it is no longer needed.
zdevito
left a comment
There was a problem hiding this comment.
Looks good now, but we should not call restoreAccurateTypeTags in unpickler if the container version is the current version. It should only get called for older formats now.
Also, this should land with the next PR in the stack so we don't end up in an intermediate state where some networks get generated with both build_intlist and restore_type_tags. It would be weird to have to continue to handle that case.
|
Looks like this PR breaks mobile builds on master. https://app.circleci.com/jobs/github/pytorch/pytorch/4638554 @driazati shall we revert? |
|
Fix is on the way #34180 |
Summary: Stacked PRs * pytorch#33474 - [jit] Remove list specializations from pickler * **pytorch#33255 - [jit] Add type tags to lists/dicts in pickle** This adds a global call to `torch.jit._pickle.restore_type_tags` for lists and dicts so that we can preserve their types after serialization. ](https://our.intern.facebook.com/intern/diff/19868637/) Pull Request resolved: pytorch#33255 Pulled By: driazati Reviewed By: xman1979, Tianshu-Bao Differential Revision: D19868637 fbshipit-source-id: 2f1826e6679a786ca209198690269f399a542c04
|
@pytorchbot retest this please |
2bcc6ca to
cc66173
Compare
Summary: Stacked PRs * pytorch#33474 - [jit] Remove list specializations from pickler * **pytorch#33255 - [jit] Add type tags to lists/dicts in pickle** This adds a global call to `torch.jit._pickle.restore_type_tags` for lists and dicts so that we can preserve their types after serialization. ](https://our.intern.facebook.com/intern/diff/19868637/) Pull Request resolved: pytorch#33255 Pulled By: driazati Reviewed By: xman1979, Tianshu-Bao Differential Revision: D19868637 fbshipit-source-id: 2f1826e6679a786ca209198690269f399a542c04
Summary: Stacked PRs * pytorch#33474 - [jit] Remove list specializations from pickler * **pytorch#33255 - [jit] Add type tags to lists/dicts in pickle** This adds a global call to `torch.jit._pickle.restore_type_tags` for lists and dicts so that we can preserve their types after serialization. ](https://our.intern.facebook.com/intern/diff/20346780/) Pull Request resolved: pytorch#33255 Pulled By: driazati Differential Revision: D20346780 fbshipit-source-id: c8534954ef4adb2e3c880401acbee30cd284f3db
Summary: This enables the serialization part of this change (the deserialization stuff is already landed pytorch#33255) Pull Request resolved: pytorch#35741 Pulled By: driazati Differential Revision: D20758124 fbshipit-source-id: e2cdefa99c3bec991491e5e967e7f1661ca7ffd9
Stacked PRs
This adds a global call to
torch.jit._pickle.restore_type_tagsforlists and dicts so that we can preserve their types after serialization.
Differential Revision: D20346780