ankane / torch.rb

Deep learning for Ruby, powered by LibTorch
Other
704 stars 30 forks source link

Fix named_modules #10

Closed nerdinand closed 4 years ago

nerdinand commented 4 years ago

Fix undefined method `include?' for nil:NilClass in Torch::NN::Module#named_modules

ankane commented 4 years ago

Hey @nerdinand, thanks for the PR! Can you paste the stack trace of the error you're seeing? memo is set two lines below the change, so I'm not sure this will fix the error.

nerdinand commented 4 years ago

Without this I'm getting:

    3: from /Users/ferdi/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/torch-rb-0.3.0/lib/torch/nn/module.rb:119:in `load_state_dict'
    2: from /Users/ferdi/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/torch-rb-0.3.0/lib/torch/nn/module.rb:119:in `each'
    1: from /Users/ferdi/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/torch-rb-0.3.0/lib/torch/nn/module.rb:121:in `block in load_state_dict'
/Users/ferdi/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/torch-rb-0.3.0/lib/torch/nn/module.rb:192:in `named_modules': undefined method `include?' for nil:NilClass (NoMethodError)
ankane commented 4 years ago

Is there a chance you removed the memo ||= Set.new in your local installation? The include? check should be on line 193 instead of 192. If that's not the issue, can you share a minimal script to reproduce?

nerdinand commented 4 years ago

For some reason, when installing torch.rb with

gem 'torch-rb', github: 'ankane/torch.rb'

then i get

/Users/ferdi/.rbenv/versions/2.7.1/lib/ruby/2.7.0/rubygems/core_ext/kernel_require.rb:92:in `require': cannot load such file -- torch (LoadError)

when i require 'torch'.

It also gives me these warnings while bundling, not sure if that means anything:

Using torch-rb 0.3.0 from https://github.com/ankane/torch.rb.git (at master@f2f4cba)
/Users/ferdi/.rbenv/versions/2.7.1/lib/ruby/2.7.0/rubygems/ext/builder.rb:165: warning: conflicting chdir during another chdir block
/Users/ferdi/.rbenv/versions/2.7.1/lib/ruby/2.7.0/rubygems/ext/builder.rb:173: warning: conflicting chdir during another chdir block

But yes, I guess you're right, I must have changed it locally. When installing 0.3.0 from rubygems it looks like this:

      # TODO return enumerator?
      def named_modules(memo: nil, prefix: "")
        ret = {}
        memo ||= Set.new
        unless memo.include?(self)
          memo << self
          ret[prefix] = self
          named_children.each do |name, mod|
            next unless mod.is_a?(Module)
            submodule_prefix = prefix + (!prefix.empty? ? "." : "") + name
            mod.named_modules(memo: memo, prefix: submodule_prefix).each do |m|
              ret[m[0]] = m[1]
            end
          end
        end
        ret
      end

(of course with the bug from #9 again)

ankane commented 4 years ago

I'm not able to reproduce the require "torch" error when installing from GitHub unfortunately. Make sure your Ruby script is configured to use Bundler.

require "bundler/setup"
require "torch"

The Bundler warnings aren't anything to worry about (not specific to Torch.rb).

nerdinand commented 4 years ago

Hmm, very interesting, that indeed in this case i need

require 'bundler/setup'

for it to work... Or indeed running with bundle exec also does the trick...

ankane commented 4 years ago

Glad that fixed it. The reason is Ruby doesn't know how to find gems installed by Bundler from GitHub without Bundler (if that makes any sense).