SciRuby / nmatrix

Dense and sparse linear algebra library for Ruby via SciRuby
Other
469 stars 133 forks source link

NMatrix to_a too slow #543

Closed lokeshh closed 6 years ago

lokeshh commented 8 years ago

While porting Statsample-GLM form Matrix to NMatrix I found to_a is taking a lot of time than it should and in fact it is as the following benchmark shows:

require 'benchmark'
require 'matrix'
require 'nmatrix'

m = Matrix.zero 1000 ;
n = NMatrix.new [1000, 1000], [0]*10**6

Benchmark.bm do |x|
  x.report "Matrix to_a" do
    10.times { m.to_a }
  end

  x.report "NMatrix to_a" do
    10.times { n.to_a }
  end
end

       user     system      total        real
Matrix to_a  0.010000   0.000000   0.010000 (  0.009627)
NMatrix to_a  4.680000   0.080000   4.760000 (  4.777082)

Improvement in to_a might solve the issue in https://github.com/SciRuby/statsample-glm/pull/36

translunar commented 8 years ago

So, looking at this, I at first thought it was interesting, but now I'm not so sure.

I believe that the Ruby Matrix class stores the data as an array. So when you call #to_a, you're basically just getting a copy of the internal storage.

NMatrix, however, has to make a new Array — first allocating and then copying values into it.

I guess the question is this: what are you doing that is requiring you to call #to_a? Can you avoid it? By its nature, it's going to involve a full copy operation no matter what.

Before we label this bug or enhancement, please have a look through the relevant code and figure out if we're using best practices already or not. It may be there's a simple mistake, or it may be that there are actual enhancements that can be made, or it may already be optimal.

gtamba commented 8 years ago

I guess the question is this: what are you doing that is requiring you to call #to_a? Can you avoid it? By its nature, it's going to involve a full copy operation no matter what.

Python data science libraries encourage converting numpy arrays to a list after vectorized computations and linear algebra functions are no longer required for massive data sets, because indexing and enumerating a numpy array that is very large is much slower than if it were a simple list, I'm not sure if this is Lokesh's use case but I've seen this a lot while I was doing a course on Information Retrieval using scikit-learn , also the tradeoff costs are not clear in our case either.