Skip to content

[WIP] Module export using custom file format#9794

Closed
jamesr66a wants to merge 4 commits intopytorch:masterfrom
jamesr66a:file_format2
Closed

[WIP] Module export using custom file format#9794
jamesr66a wants to merge 4 commits intopytorch:masterfrom
jamesr66a:file_format2

Conversation

@jamesr66a
Copy link
Collaborator

Stacked on #9746. Look at the last commit.

This implements module (de)serialization using a custom aligned data format

@ezyang
Copy link
Contributor

ezyang commented Jul 24, 2018

@pytorchbot retest this please

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.

Generally the on-disk format looks right, but the APIs for reading/writing need serious cleanup. We need to get them right otherwise the ways we will try to improve them won't be possible.

// string verisions of their file offsets. This also finalizes the file,
// and calling serializeTensor after calling this method is illegal.
// NOTE: this method mutates the model proto
size_t serializeModelProto(::ONNX_NAMESPACE::ModelProto *model_proto) {

This comment was marked as off-topic.

This comment was marked as off-topic.

}

// Serialize a tensor to file, then return its offset
size_t serializeTensor(const std::string& name, at::Tensor t) {

This comment was marked as off-topic.

This comment was marked as off-topic.

padToNextAlignmentBoundary();
}

void swapTensorAttributeNames(::ONNX_NAMESPACE::GraphProto *g) {

This comment was marked as off-topic.

readAndValidateFileHeader();
}

// returns raw data map and the index of the last record (i.e. the ModelProto)

This comment was marked as off-topic.

}

// returns raw data map and the index of the last record (i.e. the ModelProto)
std::tuple<std::unordered_map<std::string, std::string>&, size_t> read_raw() {

This comment was marked as off-topic.

~PyTorchFileReader() {
std::fclose(fp);
}

This comment was marked as off-topic.

This comment was marked as off-topic.

return std::make_tuple(model_proto.SerializeAsString(), graph_encoder.get_raw_data_export_map());
}

class PyTorchFileWriter {

This comment was marked as off-topic.

}, py::arg("onnx_opset_version")=0,
py::arg("operator_export_type")=::torch::onnx::OperatorExportTypes::RAW)
.def("export_to_pytorch_file", [](const std::shared_ptr<Module> m, const std::string& filename,
int64_t onnx_opset_version,

This comment was marked as off-topic.

@jamesr66a
Copy link
Collaborator Author

Starting new PR soon

@jamesr66a jamesr66a closed this Jul 25, 2018
facebook-github-bot pushed a commit that referenced this pull request Jul 28, 2018
Summary:
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
Pull Request resolved: #9900

Reviewed By: zdevito

Differential Revision: D9021057

Pulled By: jamesr66a

fbshipit-source-id: 01af74a7fdd1b90b2f5484644c3121d8ba9eb3b3
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants