Skip to content
This repository was archived by the owner on Oct 13, 2021. It is now read-only.

Handle time_major in lstm and remove reshape from embedding #457

Merged
merged 2 commits into from
Apr 24, 2020

Conversation

sonu1-p
Copy link
Contributor

@sonu1-p sonu1-p commented Apr 23, 2020

This commit makes the following changes:

  1. Embedding was unnecessarily reshaping input to a rank2 tensor. This
    is not required and causes model to diverge from what its actually
    supposed to do.
  2. Lstm was not considering time_major param. No transpose of input and
    output is required if the input is already time major, i.e of shape
    [ seq_len, batch_size, input_size]
  3. Simplify the handling of case when return sequences is set to true.
  4. Use squeeze and unsqueeze instead of reshape for dim representing
    direction of lstm for simplicity.

sonu1-p and others added 2 commits April 23, 2020 16:10
This commit makes the following changes:
1. Embedding was unnecessarily reshaping input to a rank2 tensor. This
   is not required and causes model to diverge from what its actually
   supposed to do.
2. Lstm was not considering time_major param. No transpose of input and
   output is required if the input is already time major, i.e of shape
   [ seq_len, batch_size, input_size]
3. Simplify the handling of case when return sequences is set to true.
4. Use squeeze and unsqueeze instead of reshape for dim representing
direction of lstm for simplicity.
@jiafatom jiafatom merged commit 79df32d into onnx:master Apr 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants