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_r279452418
 
 

 ##########
 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:
   Nevermind I misunderstood earlier.
   
   Actually the `states` array is *possibly* a user-provided parameter, but if 
they don't provide it then the code will fill in a default value.
   
   I was having trouble getting this to work:
   ```
   model(input, sequence_length=sequence_length)
   ```
   Because the code kept on complaining about an unknown parameter 
`sequence_length`. I think this must be related to how gluon handles keyword 
arguments for a model's forward pass.
   
   The solution I found was to over-ride the `__call__()` method for the rnn 
layer which is happy to take keyword arguments, and then I can use that to 
impose an order for arguments that I pass to the `hybrid_forward()` function. 
To get this to work I also needed to move the logic of setting a default value 
for `states` into the `__call__()` function.

----------------------------------------------------------------
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