Skip to content

Conversation

@jamesr66a
Copy link
Collaborator

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

@jamesr66a jamesr66a force-pushed the file_format_simple branch 4 times, most recently from 17dd7bc to 024f7c9 Compare July 26, 2018 20:17
@jamesr66a jamesr66a force-pushed the file_format_simple branch from 024f7c9 to a3713e0 Compare July 26, 2018 20:44
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.

This looks good. Is there a reason we cannot return std::shared_ptr?

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.

return getRecordWithKey(last_record_offset);
}
std::string getRecordWithKey(uint64_t key) {
if (key > file_size) {

This comment was marked as off-topic.


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.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a 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.

@ezyang
Copy link
Contributor

ezyang commented Jul 26, 2018

@pytorchbot retest this please

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a 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.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a 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.

@jamesr66a jamesr66a added the oncall: jit Add this issue/PR to JIT oncall triage queue label Jul 27, 2018
@jamesr66a jamesr66a force-pushed the file_format_simple branch from 6fbbb04 to 2350ec0 Compare July 27, 2018 00:52
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a 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.

@jamesr66a
Copy link
Collaborator Author

@pytorchbot retest this please

Copy link
Collaborator

@dzhulgakov dzhulgakov left a 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.

// Utility functions
uint64_t read64BitIntegerLittleEndian() {
uint64_t retval;
// TODO endian swap on platforms that need it?

This comment was marked as off-topic.

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.

}
} // namespace

class PyTorchFileReader {

This comment was marked as off-topic.

jramseyer pushed a commit to jramseyer/pytorch that referenced this pull request Jul 30, 2018
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
goodlux pushed a commit to goodlux/pytorch that referenced this pull request Aug 15, 2018
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
@ezyang ezyang added the merged label Jun 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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