-
Notifications
You must be signed in to change notification settings - Fork 26.3k
PyTorch File Format API #9900
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
PyTorch File Format API #9900
Conversation
17dd7bc to
024f7c9
Compare
024f7c9 to
a3713e0
Compare
zdevito
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good. Is there a reason we cannot return std::shared_ptr?
torch/csrc/jit/serialization.h
Outdated
| std::string getLastRecord() { | ||
| return getRecordWithKey(last_record_offset); | ||
| } | ||
| std::string getRecordWithKey(uint64_t key) { |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
torch/csrc/jit/serialization.h
Outdated
| return getRecordWithKey(last_record_offset); | ||
| } | ||
| std::string getRecordWithKey(uint64_t key) { | ||
| if (key > file_size) { |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
torch/csrc/jit/serialization.h
Outdated
|
|
||
| void writePad(const size_t num_bytes) { | ||
| uint8_t pad_val = kPadValue; | ||
| // TODO is there a more efficient way to do this? |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
facebook-github-bot
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jamesr66a has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@pytorchbot retest this please |
facebook-github-bot
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jamesr66a has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
facebook-github-bot
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jamesr66a has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
6fbbb04 to
2350ec0
Compare
facebook-github-bot
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jamesr66a has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@pytorchbot retest this please |
dzhulgakov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks legit :)
| } | ||
| private: | ||
| FILE *fp; | ||
| size_t cursor = 0; |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
| // Utility functions | ||
| uint64_t read64BitIntegerLittleEndian() { | ||
| uint64_t retval; | ||
| // TODO endian swap on platforms that need it? |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
| std::shared_ptr<void> data; | ||
| size_t size; | ||
| std::tie(data, size) = self.getRecordWithKey(key); | ||
| return py::bytes(reinterpret_cast<const char*>(data.get()), size); |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
| } | ||
| } // namespace | ||
|
|
||
| class PyTorchFileReader { |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
Summary: This is a follow-up to pytorch#9794 that contains only the serialization library and exposes a cleaner API. This should later be incorporated into the module export code Pull Request resolved: pytorch#9900 Reviewed By: zdevito Differential Revision: D9021057 Pulled By: jamesr66a fbshipit-source-id: 01af74a7fdd1b90b2f5484644c3121d8ba9eb3b3
Summary: This is a follow-up to pytorch#9794 that contains only the serialization library and exposes a cleaner API. This should later be incorporated into the module export code Pull Request resolved: pytorch#9900 Reviewed By: zdevito Differential Revision: D9021057 Pulled By: jamesr66a fbshipit-source-id: 01af74a7fdd1b90b2f5484644c3121d8ba9eb3b3
This is a follow-up to #9794 that contains only the serialization library and exposes a cleaner API. This should later be incorporated into the module export code