LudovicoYIN commented on PR #19634: URL: https://github.com/apache/tvm/pull/19634#issuecomment-4571344781
Thanks for the detailed review. I updated the sequence recurrent converters to address the correctness issues you pointed out. 1. For both sequence LSTM converters, the fused activation is now applied on the internal cell path rather than as a post-activation on the final hidden state. I also fixed the `time_major=True` layout handling so the output preserves `[time, batch, ...]`. 2. For unsupported optional inputs, the converters now explicitly raise `OpNotImplemented` instead of silently ignoring peephole / projection / layer-norm / aux-input variants. I also strengthened the tests: - `BIDIRECTIONAL_SEQUENCE_RNN` now uses the full 12-input form - `BIDIRECTIONAL_SEQUENCE_LSTM` now uses the full 48-input form - added `time_major=True` regression coverage - added unsupported-input rejection tests - added stronger equation/activation-level checks for the recurrent lowering -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
