search

Element-Research / rnn

Recurrent Neural Network library for Torch7's nn
BSD 3-Clause "New" or "Revised" License
939 stars 313 forks source link

add grid lstm #345

Open christopher5106 opened 7 years ago

christopher5106 commented 7 years ago

I added 2D Grid LSTM following https://github.com/coreylynch/grid-lstm It is pretty slow and I get an out of memory error. Sounds I'm not sure to understand all aspects of Element-research/rnn...

christopher5106 commented 7 years ago

An example of usage of the layer nn.Grid2DLSTM https://github.com/christopher5106/grid-lstm

christopher5106 commented 7 years ago

But it works.

It is more a question on what is happening behind the element research framework... to get it work as fast as the original version, without memory leak.

Could someone advise me on these questions :

1° how does garbage collection work ? training multiple forward / backwards does delete tensors ? shall I call forget method during each step of the training ?

2° if I want to initialize the parameters (https://github.com/christopher5106/grid-lstm/blob/master/train.lua#L163-L180) inside the layer, in which function shall I put it ?

Thanks a lot

christopher5106 commented 7 years ago

Using rnn:forget() solves the memory and speed issue.

christopher5106 commented 7 years ago

Small corrections. Works perfectly now.

nicholas-leonard commented 7 years ago

@christopher5106 Is it too l ate for me to ask you to include documentation and unit test? Sorry for the delay.

ddovod commented 7 years ago

Hello guys! Any updates on this PR? Looks tasty!

christopher5106 commented 7 years ago

What would you like exactly for this?

nicholas-leonard commented 7 years ago

@christopher5106 For documentation, adding a section to README.md with link to paper and brief explanation should do the trick. For unit tests, add a function to test.lua to make sure GridLSTM behaves as expected. Doesn't have to be extensive.