Flatten equations for rnn patterns

[buddhapuneeth]

In rnn_utils.py file, we can see all the regex of the LSTM and GRU nodes. It will be helpful if you add the actual mathematical equation also there as everyone follow different notations, so it will be more readable.

[buddhapuneeth]

Also we have operator type 'Enter' in the pattern, explanation of that operator is not available anywhere.

[jignparm]

These op patterns are what are observed in TF graphs, with 'Enter' and 'Exit' denoting flow-control operators. Most of the patterns in rnn_utils.py should read like a textbook (e.g. see below for a sequence of ConcatV2, MatMul, BiasAdd etc.). As for mathematical equations, that could be a nice touch, although that could apply to almost every other (regular) operator as well.

Is there some model that's not working/converting?

# TensorFlow LSTMCell/BasicLSTMCell computation graph matching
xc_pattern = OpTypePattern('Split', inputs=[
    OpTypePattern("Const"), # axis for split
    OpTypePattern("BiasAdd", name="bias_add", inputs=[
        OpTypePattern("MatMul", inputs=[
            OpTypePattern("ConcatV2|Concat", name="xh"),
            OpTypePattern("Enter", inputs=[
                OpTypePattern("*", name="cell_kernel"),
            ]),
        ]),
        OpTypePattern("Enter", inputs=[
            OpTypePattern("*", name="cell_bias"),
        ]),
    ]),
])

[buddhapuneeth]

Thanks for explaining on Enter and Exit operators.
Yes, I am facing issue while converting LSTM models. The sub-graph pattern matching is failing.
It was working after making small change as below (by skipping Enter):

xc_pattern = OpTypePattern('Split', inputs=[
  OpTypePattern("Const"), # axis for split
  OpTypePattern("BiasAdd", name="bias_add", inputs=[
      OpTypePattern("MatMul", inputs=[
          OpTypePattern("ConcatV2|Concat", name="xh"),
          OpTypePattern("*", name="cell_kernel"),
      ]),
      OpTypePattern("*", name="cell_bias"),
  ]),
])

This is working but need to understand why Enter operator is not coming.

[guschmue]

It basically matches the lowered tensorflow control flow and transforms it to something higher so we can map it to onnx ops like lstm or loop. Since this is part of the tensorflow graph it should be documented there but while ops like Enter are documented the concept itself is best explained here: https://arxiv.org/pdf/1805.01772.pdf

I don't think you can remove the Enter from the pattern since Enter indicates the start of the control flow. Maybe another op behind the Enter prevents the matching to work?
Which tensorflow version are you using ?

[buddhapuneeth]

I am using TF 1.15.0 version. I am sure that after removing Enter, graph pattern is matching with LSTM.

[guschmue]

We definitely had issues with tf-1.15 and made changes in the master to get unit tests to work on it.
With the Enter removed, are you end to end results correct ?
We think we have unit test coverage for the common lstm's here https://github.com/onnx/tensorflow-onnx/blob/master/tests/test_lstm.py and they are happy on tf-1.15, Anything special for your lstm?

[buddhapuneeth]

With Enter removed, it is working for us. I made comparison by running the generated onnx file on onnx runtime and pb file on tf runtime, results in both the cases are same.
I feel test cases for LSTM are buggy, as we are not checking for LSTM nodes in the resultant output graph. It is commented:
https://github.com/onnx/tensorflow-onnx/blob/master/tests/common.py#L329

When I debug, I am getting 0 LSTM nodes for those tests.
Is there any recommended TF version for tf2onnx?

[guschmue]

oh, I see - we just execute the ops? I can take a look at it.
tf-1.14 is good, In tf-1.15 bunch of stuff has changed, guess because it is the bridge to tf-2.0.

[buddhapuneeth]

Yes. Assertion is not done in unit tests.
I tried with tf-1.14, these are my observations:

• LSTM Unit tests are passing (locally verifying the number of nodes)
• Working without any change in xc_pattern (I mean with Enter operator)
  So it is evident that there are some issues with tf-1.15. Unfortunately, we need to work on tf-1.15. Any suggested workarounds in this case?

[guschmue]

I need to find some time to debug it, should be getting to it this week.

[buddhapuneeth]

@guschmue I just want to check is there any update? or any tentative timeline for complete TF-1.15 support?

[guschmue]

I kind of know where the issue comes from. Trying to find time to fixing it but need to finish some other task first.

[buddhapuneeth]

Thanks @guschmue for the update.
Please update me once it is done. We have some version dependencies, it will be a great help.

[buddhapuneeth]

@guschmue any update on this.