New serialization format

[houseroad]

Addressed Dima's feedback.

The proposal is here: https://fb.quip.com/TbQmAuqIznCf

[facebook-github-bot]

dzhulgakov has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[zdevito]

I'd like to see a better overall picture about where this is going in the long term. When we are done merging the formats, what do we want the new format to be?

The format as-is seems like a mishmash of random fields needed by caffe2's current model and a few places string for where a future torch models format would fit in. It feels like it is code that we will have to clean up in the future. There are tons of Argument and NetDef fields sitting throughout the format. Some input/output fields in MethodDef will be redundant with how torch represents networks. I also do not see how Tensors will be stored with proper storages where two tensors can be views of the same storage. This is an important part of the PyTorch format.

I'd like to get this to the point where the core structure reflects the ideal we are working toward that will be shared by all of our serialization formations, with the per-backend fields put inside sub objects. Otherwise it is very hard to see what can be shared among formats vs what is only filled in for a specific format.

[houseroad]

Great thanks to @zdevito for the valuable input! I agree with that we should have a clean design at this moment instead of cleaning it up in the future. I will remove the unnecessary fields, make sure that from the organization side, nothing looks redundant.

At high level, since right now we need to support both static graph and torch script, we aim unifying the model format (proto), APIs and tooling. Later when everything is ready, we can start unifying the methods.

[facebook-github-bot]

houseroad has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[dzhulgakov]

For the OpDef changes - they probably require the longer discussion on input/argument distinction and what's the longer term migration path. Before that discussion adding random fields might be premature. So if you want to proceed with other parts of this diff (modules and tensors) - maybe commenting out and keeping NetDef as is mostly is best.

[facebook-github-bot]

houseroad has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[houseroad]

Thanks a lot for the valuable inputs from @Yangqing @dzhulgakov @kennyhorror .

I have made the following changes:

1. premature design are removed, i.e., named_inputs/outputs and Non-tensor type support
2. unnecessary fields are removed, e.g., debug_info in multiple structures
3. extract external data into a separate structure, strides and id/path are binded to ExternalData
4. add another layer of abstract - ParameterDef, and move is_buffer and requires_gradient to it
5. more comments are added

[facebook-github-bot]

houseroad has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[facebook-github-bot]

houseroad has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[dzhulgakov]

minor naming tweaks and ship it

[facebook-github-bot]

houseroad has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[Yangqing]

Maybe a rebase and see if circleCI passes? It is showing weird failure at the moment.

[facebook-github-bot]

houseroad has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[houseroad]

Yeah, I guess some docker images are updated and this caused the problems. Just rebased the PR, and let's see.