stephenrawls commented on a change in pull request #14208: Add support for fast 
variable-length LSTM
URL: https://github.com/apache/incubator-mxnet/pull/14208#discussion_r279226947
 
 

 ##########
 File path: python/mxnet/gluon/rnn/rnn_layer.py
 ##########
 @@ -214,7 +215,7 @@ def begin_state(self, batch_size=0, func=ndarray.zeros, 
**kwargs):
             states.append(func(name='%sh0_%d'%(self.prefix, i), **info))
         return states
 
-    def hybrid_forward(self, F, inputs, states=None, **kwargs):
+    def hybrid_forward(self, F, inputs, sequence_length=None, states=None, 
**kwargs):
 
 Review comment:
   I think `sequence_length` actually has to come before `states`, because it 
is a positional argument provided by the user, whereas `states` is an automatic 
positional argument provided by gluon, since gluon automatically adds a block's 
learnable parameters as positional arguments at the end of  `hybrid_forward()` 
functions' argument list.
   
   The reason my personal unit tests were passing was because I was using a 
version of my code that hadn't switched the order of these arguments. I 
reverted this change so hopefully the centsos-gpu tests will pass now.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

Reply via email to