Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Masking is a intra-layer behavior in TF LSTM [1] but is not a intra-op behavior in ONNX LSTM [2]. When converted to ONNX, masked TF LSTM layer is converted to Loop op. This over-complicates the ONNX model, and has a negative impact on inference performance in ORT without leveraging LSTM optimizations. (issue #1871)
This commit adds support to convert masked LSTM correctly, under the important assumption that input must be post-padded - which is the most common use case. The "masking" info is conveyed to ONNX LSTM op as
sequence_lens
which is dynamically computed by summing the number of non-skip timesteps per batch per-LSTM. This behavior is implemented with reference to keras2onnx PR#386 [3]. Additional logic is added for backward LSTM so that the input sequence is reversed correctly givensequence_lens
.Note that if mask-enabled, and LSTM input is pre- or randomly padded, the converted ONNX model will behave incorrectly for inference. Unless ONNX add new attribute e.g.
mask_enabled
to RNN ops, converter alone may not be able to handle generic masking while keeping the RNN ops, since masking alters intra-op behavior. With such limitation, I'd like to share this PR for further comment and suggestion.[1] https://www.tensorflow.org/guide/keras/masking_and_padding#masking
[2] https://github.com/onnx/onnx/blob/main/docs/Operators.md#LSTM
[3] onnx/keras-onnx#386
Details:
Forward LSTM
Here's an minimal example with an embedded LSTM (
mask_zeros=True
):Reverse LSTM
tf.raw_op.ReverseV2
->ReverseSequence
behavior to reverse LSTM input correctly: