Element-Research / rnn

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

rnn init.lua contains classes that belong to dpnn #427

Closed tastyminerals closed 6 years ago

tastyminerals commented 6 years ago

I have just reinstalled Torch with its corresponding rnn and dpnn packages and my model call th main.lua which ran without issues before started throwing the following errors:

/home/pavel/torch/install/bin/luajit: /home/pavel/torch/install/share/lua/5.1/trepl/init.lua:389: 

/home/pavel/torch/install/share/lua/5.1/trepl/init.lua:389: 

/home/pavel/torch/install/share/lua/5.1/torch/init.lua:102: class nn.SpatialGlimpse has been already assigned a parent class

stack traceback:
    [C]: in function 'error'
    /home/pavel/torch/install/share/lua/5.1/trepl/init.lua:389: in function 'require'
    main.lua:4: in main chunk

or after commenting out require('rnn.SpatialGlimpse') from init.lua:

/home/pavel/torch/install/bin/luajit: /home/pavel/torch/install/share/lua/5.1/trepl/init.lua:389: 

/home/pavel/torch/install/share/lua/5.1/trepl/init.lua:389: 

/home/pavel/torch/install/share/lua/5.1/torch/init.lua:102: class nn.ArgMax has been already assigned a parent class

stack traceback:
    [C]: in function 'error'
    /home/pavel/torch/install/share/lua/5.1/trepl/init.lua:389: in function 'require'
    main.lua:4: in main chunk
    [C]: in function 'dofile'
    ...avel/torch/install/lib/luarocks/rocks/trepl/scm-1/bin/th:150: in main chunk
    [C]: at 0x00405b70

I checked recent commits but could not find whether somebody modified rnn init.lua so I guess something went wrong during installation via luarocks.

UPDATE: It looks like /home/pavel/torch/install/share/lua/5.1/rnn/init.lua is different from what is currently in master... but why? I performed fresh Torch installation via ./install.sh and then I installed rnn and dpnn via ~/torch/install/bin/luarocks binary same way I did many times before.

UPDATE: I managed to run my model by commenting out require "dpnn" from main.lua. This is obviously a temp solution. The dpnn and rnn init.lua files contain similar class instantiation calls if Torch is installed by default and looks like this issue can be exposed only if dpnn is explicitly imported together with rnn.

@nicholas-leonard, @jnhwkim can you please check this out whenever you have a spare minute?

tastyminerals commented 6 years ago

Ok, ba937a08f26e116db98b5cd6a690f49ea8f8316e commit explains everything. Merging of Element-Research to torch/rnn. So, now if you install Torch, luarocks will be pulling files from torch/rnn which contains a lot of changes and will break your models if they were based on Element-Research/rnn branch, so in order to prevent this either refactor your code (which is not a solution at least for me because there is a lot, a lot of work) or continue using this repo.

So, if you one of those ppl who decided that it is a good time to update your Torch distro and found out that your older code does not work anymore do the following:

git clone https://github.com/Element-Research/rnn.git
cd rnn
luarocks make rocks/rnn-scm-1.rockspec

This should fix your problems. But you eventually do need to refactor your models or any custom layers you've implemented. Thank you devs!

choas commented 6 years ago

@tastyminerals shouldn't it be? luarocks make rocks/rnn-scm-1.rockspec