Add design doc for onnx convertor #9296

kuke · 2018-03-21T10:45:16Z

Resolve #9297

varunarora · 2018-03-22T19:23:35Z

Nice start! Let's keep the momentum going. Things I would love for this document to address before focusing on software design details like how to organize modules and commands to run:

Work needed / issues to take Fluid model blocks / ProgramDesc and turn it into the sequence of nodes needed by ONNX
How we are going to map ops and their inputs and outputs
Issues with types and how we may address them
Possible inspiration from other implementations like PyTorch
Versioning
Unsupported fluid ops and deprecation plan

Also, I didn't hear @wangkuiyi say we needed bi-directional conversion. Can we just focus on fluid to ONNX for now? ONNX to Fluid seems a little unimportant right now. Kindly correct me if I am wrong.

varunarora · 2018-03-22T19:38:52Z

And perhaps a minimal set of models for support for v1.

varunarora · 2018-03-22T23:44:27Z

Update: I read this #9108 again and it seems like being "ONNX compatible" is a focus. I still think we should first aim to finish fluid-to-ONNX first. But yes, this doc can contain design considerations for onnx-on-fluid also.

varunarora · 2018-03-23T01:13:48Z

So I am sharing some info on behalf of @sidgoyal78 and myself. We did our part of thinking and reviewing of ONNX tools and supported framework utils. Here are a few notes we made during the process:

Most of the construction of the ONNX graph can be accomplished by the Python helper.py utility, as described in this example: https://github.com/onnx/onnx/blob/master/onnx/examples/Protobufs.ipynb. We obviously need to wrap these helpers for Paddle ops.
We need to come up with a mapping of types for Paddle types
Read some of the gotchas of TensorFlow to ONNX conversation here: https://github.com/onnx/tensorflow-onnx#how-tf2onnx-works
Each op needs to written custom mappers for, to map to ONNX ops. Inputs and outputs don't necessarily map from Paddle to ONNX - in that, in some cases, inputs from Paddle may be attributes into ONNX ops. Before diving into working on each mapping, notes need to be written on this (possibly as comments inside the module where these are implemented).
This also needs to be done to decompose complex ops. Prior to implementation.
To start, we can focus on ensuring coverage and "runnability" of a select set of models, starting from more straightforward ones to more complex ones. We could use our models repo or the book to select the list of these.

pkuyym · 2018-03-23T10:06:08Z

@varunarora Thanks for your valuable comments and we are totally agree with your suggestions. @kuke is focus on remain sections and please feel free to add your opinions. @kuke Could you invite @varunarora and @sidgoyal78 be the co-contributor for this PR?

Currently, let's focus on the conversion from Fluid to ONNX and leave reverse conversion considered in future.
Since time is limited before the first DDL (End April), I suggest that we only consider conversion for models without control flow operators and sequence models not based on Tensor with LoD. Now, ONNX emphasizes acyclic graph, so involving WhileOp/IfElseOp may make things difficult and out of control. LoDTensor is particular feature in Fluid and it's not easy to convert related ops to ONNX format.
I think this repo is a good reference https://github.com/onnx/onnxmltools.
Building a minimal prototype first would help us be more clear about the details. I suggest we can implement such prototype ASAP and let's make sure this work finished in less than several days like 2 or 3 days.

kuke · 2018-03-23T12:31:09Z

Hi @varunarora and @sidgoyal78, I have invited you as the collaborators of this pull request (https://github.com/kuke/Paddle/tree/onnx). Please feel free to edit this draft directly if you have any ideas. Let's complete this design doc together ASAP. Finally we invite others to have a review.

varunarora · 2018-03-23T18:14:41Z

Building a minimal prototype first would help us be more clear about the details. I suggest we can implement such prototype ASAP and let's make sure this work finished in less than several days like 2 or 3 days.

Me and @sidgoyal78 are on the same page! Let's make it happen.

abhinavarora · 2018-03-28T00:15:44Z

doc/fluid/design/onnx/onnx_convertor.md

+
+* **fluid**: Contain wrappers for fluid related APIs. Fluid has provided some low-level APIs to parse or generate the inference model. However, directly using these low-level APIs makes the code tediously long. This module wraps low-level APIs to provide simplied interfaces.
+
+* **onnx**: ONNX uses proto file to save computation flow and model weights. This module is responsible for parsing and generating ONNX binary model.


generating ONNX binary model. -> generating a ONNX binary model.

abhinavarora · 2018-03-28T00:17:49Z

doc/fluid/design/onnx/onnx_convertor.md

+
+* **onnx**: ONNX uses proto file to save computation flow and model weights. This module is responsible for parsing and generating ONNX binary model.
+
+* **onnx_fluid**: Concepts in fluid like program, block etc. haven't direct corresponding concepts in ONNX. Even that both contains operator concept, for many operators adaption is also necessary. This module is the most important module responsible for acutal converting. Adaption for different level concepts should be provided like fluid program/block to ONNX graph, fluid operators to ONNX operators etc.


This should be as follows:

Concepts in fluid like program, block etc. do not have any direct corresponding concepts in ONNX. Even though both contain the operator concept, for many operators adaptation is also necessary. This module is the most important module and is responsible for the actual conversion. Adaption for different level of concepts should be provided like fluid program/block to ONNX graph, fluid operators to ONNX operators etc.

abhinavarora

Thank yo for the PR. There are some grammatical issues. Please fix them before merging.

sidgoyal78 · 2018-03-28T04:23:30Z

@abhinavarora : I think this was a WIP pull request, so naturally, it isn't complete as of now.

varunarora · 2018-04-21T02:04:30Z

@kuke I think we should be ready for a review here

kuke

Two minor comments. Yeah, I think we can invite @wangkuiyi to review now.

kuke · 2018-04-21T03:10:57Z

doc/fluid/design/onnx/onnx_convertor.md

+
+Therefore, it is necessary to enable the conversion between PaddlePaddle and ONNX. This design doc is aimed at implementing a convertor, mainly for converting between **Fluid** models and ONNX (it is very likely that we may support older v2 models in the future). A complete convertor should be bidirectional - with a frontend AND a backend, but considering the importance, the we will start with the frontend i.e. Fluid models to ONNX models.
+
+One thing that makes it doable in Fluid's case is the use of a static IR - the `ProgramDesc` - as opposed to a dynamic graph, as created in the cases of frameworks like PyTorch.


Should we move this line to the end of How it works?

kuke · 2018-04-21T03:27:25Z

doc/fluid/design/onnx/onnx_convertor.md

+
+```
+python convert.py --fluid_model <fluid inference model> --onnx_model <ONNX model> validate True
+```


It is better to separate conversion and validation, and we did in the main repo. So line 48~52 should be like:

Convert Fluid inference model to ONNX binary model

python convert.py --fluid_model <fluid inference model> --onnx_model <ONNX model>

Validate the converted model

python validate.py --fluid_model <fluid inference model> --onnx_model <ONNX model>

Ha yes, I caught this too but let it pass since this was a design doc. Fixed now :)

kuke · 2018-04-24T12:44:51Z

@varunarora Thanks for the update.

wangkuiyi · 2018-04-24T17:15:19Z

doc/fluid/design/onnx/onnx_convertor.md

+* Convert Fluid inference model to ONNX binary model
+
+    ```
+    python convert.py --fluid_model <fluid inference model> --onnx_model <ONNX model> validate True


convert.py => fluid_to_onnx.py

OK, we will change the script name in main repo, then here. There are some tiny inconsistencies in implementation already, so later we'll modify this design doc again.

wangkuiyi · 2018-04-24T17:17:02Z

doc/fluid/design/onnx/onnx_convertor.md

+The conversion and model validation will be completed consecutively, finally output a readable model structure description. And for the converse conversion, users only need to exchange the input and output.
+
+
+# Challenges and mitigation


Great points. Let's move a step forward -- after this PR is merged, let us consider to list some models (maybe from the Paddle Book) which could be trained and exported to the ONNX format, and doing an application-driving work, which should allow us tracking the progress easily. Also, I suppose that the progress would reveal more compatibility issues.

Yes. We have listed some models to be supported at first stage, and things are going as the plan.

kuke

Thanks for the review. Let's merge it first and refine it later.

kuke · 2018-04-25T02:17:47Z

doc/fluid/design/onnx/onnx_convertor.md

+* Convert Fluid inference model to ONNX binary model
+
+    ```
+    python convert.py --fluid_model <fluid inference model> --onnx_model <ONNX model> validate True


OK, we will change the script name in main repo, then here. There are some tiny inconsistencies in implementation already, so later we'll modify this design doc again.

kuke · 2018-04-25T02:21:19Z

doc/fluid/design/onnx/onnx_convertor.md

+The conversion and model validation will be completed consecutively, finally output a readable model structure description. And for the converse conversion, users only need to exchange the input and output.
+
+
+# Challenges and mitigation


Yes. We have listed some models to be supported at first stage, and things are going as the plan.

Fixed grammatical issues

Init onnx convertor design doc

f754a86

kuke requested a review from pkuyym March 21, 2018 10:45

pkuyym assigned kuke and pkuyym Mar 21, 2018

kuke changed the title ~~Init onnx convertor design doc~~ [WIP] Add design doc for onnx convertor Mar 21, 2018

kuke removed the request for review from pkuyym March 21, 2018 10:54

Add content for Poject Structure and Usage section.

be9977d

abhinavarora reviewed Mar 28, 2018

View reviewed changes

abhinavarora previously requested changes Mar 28, 2018

View reviewed changes

Yibing Liu and others added 3 commits March 27, 2018 22:22

Add background and other sections

f565e5d

Adding some challenges / mitigations to ONNX conversion

5550bd1

Merge branch 'develop' of upstream into onnx

b798e06

kuke mentioned this pull request Apr 12, 2018

Revise the design again for final review PaddlePaddle/Paddle2ONNX#6

Closed

Varun Arora added 2 commits April 20, 2018 18:59

Update design doc based on early implementation

56b8784

Merge branch 'onnx' of https://github.com/kuke/Paddle into onnx

718f154

kuke commented Apr 21, 2018

View reviewed changes

kuke requested a review from wangkuiyi April 21, 2018 03:39

kuke assigned varunarora Apr 21, 2018

Minor fixes based on @kuke's feedback

4766864

wangkuiyi approved these changes Apr 24, 2018

View reviewed changes

kuke commented Apr 25, 2018

View reviewed changes

kuke merged commit ee0497c into PaddlePaddle:develop Apr 25, 2018

kuke changed the title ~~[WIP] Add design doc for onnx convertor~~ Add design doc for onnx convertor Apr 26, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add design doc for onnx convertor #9296

Add design doc for onnx convertor #9296

kuke commented Mar 21, 2018 •

edited

varunarora commented Mar 22, 2018

varunarora commented Mar 22, 2018

varunarora commented Mar 22, 2018

varunarora commented Mar 23, 2018

pkuyym commented Mar 23, 2018 •

edited

kuke commented Mar 23, 2018

varunarora commented Mar 23, 2018

abhinavarora Mar 28, 2018

abhinavarora Mar 28, 2018

abhinavarora left a comment

sidgoyal78 commented Mar 28, 2018

varunarora commented Apr 21, 2018

kuke left a comment

kuke Apr 21, 2018

varunarora Apr 23, 2018

kuke Apr 21, 2018 •

edited

varunarora Apr 23, 2018

kuke commented Apr 24, 2018

wangkuiyi Apr 24, 2018

kuke Apr 25, 2018

wangkuiyi Apr 24, 2018

kuke Apr 25, 2018

kuke left a comment

kuke Apr 25, 2018

kuke Apr 25, 2018


		* fluid: Contain wrappers for fluid related APIs. Fluid has provided some low-level APIs to parse or generate the inference model. However, directly using these low-level APIs makes the code tediously long. This module wraps low-level APIs to provide simplied interfaces.

		* onnx: ONNX uses proto file to save computation flow and model weights. This module is responsible for parsing and generating ONNX binary model.


		* onnx: ONNX uses proto file to save computation flow and model weights. This module is responsible for parsing and generating ONNX binary model.

		* onnx_fluid: Concepts in fluid like program, block etc. haven't direct corresponding concepts in ONNX. Even that both contains operator concept, for many operators adaption is also necessary. This module is the most important module responsible for acutal converting. Adaption for different level concepts should be provided like fluid program/block to ONNX graph, fluid operators to ONNX operators etc.


		Therefore, it is necessary to enable the conversion between PaddlePaddle and ONNX. This design doc is aimed at implementing a convertor, mainly for converting between Fluid models and ONNX (it is very likely that we may support older v2 models in the future). A complete convertor should be bidirectional - with a frontend AND a backend, but considering the importance, the we will start with the frontend i.e. Fluid models to ONNX models.

		One thing that makes it doable in Fluid's case is the use of a static IR - the `ProgramDesc` - as opposed to a dynamic graph, as created in the cases of frameworks like PyTorch.

		The conversion and model validation will be completed consecutively, finally output a readable model structure description. And for the converse conversion, users only need to exchange the input and output.


		# Challenges and mitigation

Add design doc for onnx convertor #9296

Add design doc for onnx convertor #9296

Conversation

kuke commented Mar 21, 2018 • edited

varunarora commented Mar 22, 2018

varunarora commented Mar 22, 2018

varunarora commented Mar 22, 2018

varunarora commented Mar 23, 2018

pkuyym commented Mar 23, 2018 • edited

kuke commented Mar 23, 2018

varunarora commented Mar 23, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abhinavarora left a comment

Choose a reason for hiding this comment

sidgoyal78 commented Mar 28, 2018

varunarora commented Apr 21, 2018

kuke left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kuke Apr 21, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kuke commented Apr 24, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kuke left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kuke commented Mar 21, 2018 •

edited

pkuyym commented Mar 23, 2018 •

edited

kuke Apr 21, 2018 •

edited