Element-Research / rnn

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

recycle/forget offset unused #397

Open achalddave opened 7 years ago

achalddave commented 7 years ago

nn.Recurrent passes an offset variable to :recycle and :forget, which doesn't seem to be used anywhere. Am I missing something, or is the offset parameter unused?

Call to :recycle: https://github.com/Element-Research/rnn/blob/ef98a97b16f55f598830293435af32b509ffc5bd/Recurrent.lua#L132

Call to :forget:https://github.com/Element-Research/rnn/blob/ef98a97b16f55f598830293435af32b509ffc5bd/Recurrent.lua#L136

Defn of :recycle: https://github.com/Element-Research/rnn/blob/ef98a97b16f55f598830293435af32b509ffc5bd/AbstractRecurrent.lua#L80

Defn of :forget: https://github.com/Element-Research/rnn/blob/ef98a97b16f55f598830293435af32b509ffc5bd/AbstractRecurrent.lua#L113

achalddave commented 7 years ago

This causes the Recurrent class to store an extra copy of the RecurrentModule, which can be extremely expensive if the module requires significant memory:

require 'rnn'
local _ = require 'moses'

local model = nn.Recurrent(nn.MulConstant(2), nn.MulConstant(3))
print('#sharedClones after init:', _.count(model.sharedClones))

local input = torch.rand(1)
model:forward(input)
model:forward(input)
print('#sharedClones after forward:', _.count(model.sharedClones)) -- 1

model:forget()
print('#sharedClones after forget:', _.count(model.sharedClones)) -- 1
model:forward(input)
model:forward(input)
print('#sharedClones after 2nd forward:', _.count(model.sharedClones)) -- 2