torch / rnn

Torch recurrent neural networks
BSD 3-Clause "New" or "Revised" License
64 stars 17 forks source link

StepLSTM moved to C #18

Closed nicholas-leonard closed 7 years ago

nicholas-leonard commented 7 years ago

Need to merge StepLSTM into SeqLSTM:

I am going to complete the above in another PR. This PR is already too big.

Here are the results of the benchmark:

fast LSTM time: 0.12144248485565 seconds    
step LSTM time: 0.073302102088928 seconds   
luarec LSTM time: 0.075324392318726 seconds 
rec LSTM time: 0.066254210472107 seconds    
luaseq LSTM time: 0.067860889434814 seconds 
seq LSTM time: 0.062430810928345 seconds    
RecLSTM-C 1.1368997046676 faster than RecLSTM-Lua   
RecLSTM 1.8329776174267 faster than FastLSTM    
SeqLSTM 1.0612421893438 faster than RecLSTM 
SeqLSTM-C 1.0869775424302 faster than SeqLSTM-Lua   
Memory test 
fast LSTM memory: 98.27904510498:2.2750110626221 MB 
step LSTM memory: 17.168065071106:2.1289348602295 MB    
rec LSTM memory: 13.374607086182:2.0407600402832 MB 
seq LSTM memory: 8.8895826339722:3.0098876953125 MB

Important lines are:

RecLSTM-C 1.1368997046676 faster than RecLSTM-Lua   
SeqLSTM-C 1.0869775424302 faster than SeqLSTM-Lua

So the C version of the SeqLSTM is about 8% faster than Lua version. We can still get some more improvements by refactoring some other stuff to C and removing it from SeqLSTM, but I want to do that in another PR.

mirandaconrado commented 7 years ago

Let me know when it's ready for review =)

mirandaconrado commented 7 years ago

Reviewing. In the meantime, might be worth changing the docs because it uses rnn.Recurrent while the code has to use nn.Recurrent. I just noticed that and it's kind of confusing to the user.