Open Mustapha-AJEGHRIR opened 1 year ago
Preparing review...
๐ฏ Main theme: Batch Support Addition
๐ PR summary: This PR adds batch support to the LSTM_AE model in the sequitur library. The changes mainly involve modifying the forward methods of the Encoder, Decoder, and LSTM_AE classes to handle batched inputs.
๐ Type of PR: Enhancement
๐งช Relevant tests added: No
โจ Focused PR: Yes, the PR is focused as all changes are related to the addition of batch support for LSTM_AE.
๐ Security concerns: No
๐ก General suggestions: The PR is generally well-structured and the changes are clear. However, it would be beneficial to add tests to verify the new functionality. Additionally, it would be good to ensure that these changes are compatible with the rest of the library, as the contributor mentioned uncertainty about this.
Persistent review updated to latest commit https://github.com/shobrook/sequitur/commit/cf0a26130961ca06a33f9f300eefb61a3bef0458
Persistent review updated to latest commit https://github.com/shobrook/sequitur/commit/cf0a26130961ca06a33f9f300eefb61a3bef0458
Hello there, I have added batch support for the LSTM_AE. I don't know if this is 100% compatible with the library, but I'm only using the LSTM_AE in one of my projects, so I decided to contribute ๐ . Thanks