Skip to content
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

Support conversion of recognize_digits models #19

Merged
merged 10 commits into from Apr 16, 2018

Conversation

kuke
Copy link

@kuke kuke commented Apr 12, 2018

Resolve #18

I add this pull request immediately because I find that the fit_a_line model is too simple. And the parameters we passed parameters to the node maker are not enough to handle complex operators such as conv2d, batch norm_op covered by the recognize_digits_conv model. So I redesigned the interface and hope that it would be convenient for everyone to the implementation of complex models.

@kuke kuke changed the title Support conversion of recognize_digits_conv model [WIP] Support conversion of recognize_digits_conv model Apr 12, 2018
@varunarora
Copy link
Collaborator

Aha! Yes, tf2onxx had to the same. Can't just do a passthrough of things. But I think this was inevitable, so I am glad we have gotten to it this way.

Copy link
Collaborator

@varunarora varunarora left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really enjoying this..

convert.py Outdated

# Append some customized arguments of the node maker here
if op.type == 'conv2d':
kernel_shape = fluid.executor.fetch_var(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is all static, we could make fetch_var available inside the ops module, so we don't have to such a conditional here

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree. We should make convert.py more clean. So I change to pass operator and scope as arguments.

@@ -281,6 +301,25 @@ def pad_op():
pass


def pool2d_op(inputs, attrs, outputs):
if attrs['global_pooling'] is False:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@kuke
Copy link
Author

kuke commented Apr 13, 2018

Verified the conversion's correctness

python validate.py --fluid_model ./extras/recognize_digits_conv.inference.model/ --onnx_model ./recognize_digits_conv.onnx
-----------  Configuration Arguments -----------
a: 0.0
b: 1.0
batch_size: 10
expected_decimal: 5
fluid_model: ./extras/recognize_digits_conv.inference.model/
onnx_model: ./recognize_digits_conv.onnx
------------------------------------------------
Inference results for fluid model:
[array([[0.03526556, 0.00531864, 0.4901957 , 0.03146124, 0.21433197,
        0.01664129, 0.09860657, 0.0232774 , 0.0488808 , 0.03602083],
       [0.08501429, 0.0067768 , 0.44258946, 0.0571987 , 0.14585115,
        0.03440049, 0.11217487, 0.02188424, 0.04691784, 0.04719224],
       [0.08955405, 0.00735261, 0.28424332, 0.02858522, 0.18232903,
        0.02950039, 0.13921854, 0.03707843, 0.08816215, 0.11397629],
       [0.0623313 , 0.00577816, 0.51687336, 0.03530243, 0.15085357,
        0.02701959, 0.07948152, 0.01347689, 0.04822892, 0.06065419],
       [0.09505039, 0.00879493, 0.27774006, 0.01832659, 0.13748896,
        0.01448278, 0.25627935, 0.03942731, 0.0796967 , 0.07271288],
       [0.11718861, 0.00460373, 0.526473  , 0.02148582, 0.10427924,
        0.01576571, 0.08279876, 0.02664964, 0.05605122, 0.04470441],
       [0.0657331 , 0.00556582, 0.27806717, 0.04050628, 0.25586566,
        0.02690179, 0.13714735, 0.08382507, 0.0529239 , 0.05346391],
       [0.10572249, 0.00510338, 0.25388333, 0.05231774, 0.2349722 ,
        0.02573268, 0.15960246, 0.04436921, 0.06631914, 0.05197732],
       [0.07063236, 0.00825579, 0.39233148, 0.0192927 , 0.2183245 ,
        0.01888983, 0.14734125, 0.03661045, 0.04431242, 0.04400918],
       [0.05180368, 0.00526981, 0.5286077 , 0.02330228, 0.17934334,
        0.01972133, 0.06492393, 0.02322024, 0.04617354, 0.05763409]],
      dtype=float32)]


Inference results for ONNX model:
Outputs(_0=array([[0.03526554, 0.00531865, 0.49019575, 0.03146119, 0.21433191,
        0.0166413 , 0.09860668, 0.02327737, 0.04888073, 0.03602083],
       [0.08501417, 0.00677679, 0.44258943, 0.05719874, 0.14585117,
        0.03440046, 0.11217497, 0.02188419, 0.04691781, 0.04719226],
       [0.08955389, 0.00735262, 0.28424323, 0.02858525, 0.18232921,
        0.0295004 , 0.13921838, 0.03707845, 0.08816212, 0.11397647],
       [0.06233123, 0.00577816, 0.51687336, 0.03530247, 0.15085363,
        0.02701959, 0.07948162, 0.01347688, 0.0482289 , 0.06065418],
       [0.09505048, 0.00879495, 0.27774018, 0.0183266 , 0.1374888 ,
        0.01448278, 0.25627935, 0.03942732, 0.07969675, 0.07271287],
       [0.11718868, 0.00460375, 0.52647275, 0.02148584, 0.10427925,
        0.0157657 , 0.08279874, 0.02664967, 0.05605123, 0.04470445],
       [0.06573316, 0.00556582, 0.27806693, 0.04050625, 0.25586566,
        0.02690177, 0.13714729, 0.08382516, 0.05292396, 0.05346398],
       [0.10572256, 0.00510339, 0.25388315, 0.05231774, 0.23497239,
        0.02573269, 0.15960251, 0.04436918, 0.06631912, 0.05197728],
       [0.07063232, 0.00825581, 0.39233148, 0.01929271, 0.21832474,
        0.01888983, 0.14734115, 0.03661038, 0.04431238, 0.0440092 ],
       [0.05180358, 0.00526981, 0.5286082 , 0.02330226, 0.17934316,
        0.01972132, 0.06492393, 0.02322023, 0.0461735 , 0.05763406]],
      dtype=float32))


The exported model achieves 5-decimal precision.

@kuke kuke requested review from pkuyym and sidgoyal78 April 13, 2018 05:32
Copy link
Author

@kuke kuke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated and this pr is ready for review

convert.py Outdated

# Append some customized arguments of the node maker here
if op.type == 'conv2d':
kernel_shape = fluid.executor.fetch_var(
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree. We should make convert.py more clean. So I change to pass operator and scope as arguments.

@kuke kuke changed the title [WIP] Support conversion of recognize_digits_conv model Support conversion of recognize_digits_conv model Apr 13, 2018
Copy link
Author

@kuke kuke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some ideas about why I make the change.

# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I add a new directory fluid here to put some modules related to Fluid but independent of ONNX.

@@ -224,8 +252,63 @@ def lppool_op():
pass


def matmul_op(inputs, attrs, outputs):
return make_node('MatMul', inputs=inputs, outputs=outputs)
def mul_op(operator, scope):
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we can see, sometimes we can't implement the operator in Fluid by only operator in ONNX. So it would be better to make the function name (mul_op here) consistent with the op's name in Fluid ('mul').

inputs, attrs, outputs = get_op_io_info(operator)

# Flatten input(X) and input(Y) into 2-D matries
x_flat_out = [inputs['X'][0] + '@flatten_0']
Copy link
Author

@kuke kuke Apr 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please pay attention to the intermediate tensors between nodes. They should be named uniquely in the ONNX graph.


- [fit_a_line](https://github.com/PaddlePaddle/Paddle/blob/develop/python/paddle/fluid/tests/book/test_fit_a_line.py)
- [machine_translation](https://github.com/PaddlePaddle/Paddle/blob/develop/python/paddle/fluid/tests/book/test_machine_translation.py)
- [recognize_digits](https://github.com/PaddlePaddle/Paddle/blob/develop/python/paddle/fluid/tests/book/test_recognize_digits.py)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another model recognize_digits_mlp also has been verified.

@kuke kuke changed the title Support conversion of recognize_digits_conv model Support conversion of recognize_digits model Apr 13, 2018
@kuke kuke changed the title Support conversion of recognize_digits model Support conversion of recognize_digits models Apr 13, 2018
Copy link
Collaborator

@sidgoyal78 sidgoyal78 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I think this looks pretty good to me.

@kuke
Copy link
Author

kuke commented Apr 16, 2018

@sidgoyal78 Thanks!

@kuke kuke merged commit 57c845a into PaddlePaddle:develop Apr 16, 2018
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.

None yet

3 participants