each_row should return 1D matrix
lokeshh opened this issue · 13 comments
I am trying the following and getting unexpected result.
I want to find the max in every row.
> matrix = NMatrix[[1, 2, 3], [4, 5, 6]]
> a.each_row.map { |row| row.max }
=> [
[
[1, 2, 3] ],
[
[4, 5, 6] ]]This is not what anyone would expect. The reason why this happens is because each_row returns a 2D array and max on 2D array find the max in every column.
Well, first of all, the way you're trying to find the max in every row is wrong.
I believe you want to make use of the #max function:
https://github.com/mohawkjohn/nmatrix/blob/master/lib/nmatrix/math.rb#L705
While I can see an argument for changing this behavior, I'm concerned about the repercussions it will have on other elements of the API.
Have you looked through the NMatrix enumerate sources to see if there isn't already a function which does this?
https://github.com/mohawkjohn/nmatrix/blob/master/lib/nmatrix/enumerate.rb
Well, first of all, the way you're trying to find the max in every row is wrong.
But any one who knows ruby would expect this to true. In fact we should support this way and remove all others because that's the place at least from my experience where Scientific Computation in Ruby can win over others. The way things work in other languages is like: One figures out what operation one need to do and then he/she goes find the function that fulfill that needs. I thought this was the only way to do things until I came across Ruby, I love the way how most of the things can be covered using just map, each, etc. Very rarely I needed to do a search on how to do certain things in Ruby. Thanks to Ruby's duck typing and functional programming abilities every math is so natural. One big dis-advantage I felt of Python is that you if go by the way things are generally done in Python you get to write very bad code which is not optimal. For example "for loops in numpy" makes things really slow. So, one needs to learn various different tricks like map and vectorize which is not inherent to the language where as with Ruby all these ideas are natural. Its just like Math with very simple rules and has a clear way to do things. Everything is an object and you can only call methods on objects. If we follow Ruby's model which I strongly suggest we do we will achieve a lot more.
In this context, yes max(1) gives me the functionality I want but its confusing. I remember having to always try an example to figure out whether 1 corresponds to rows or columns. This is the not the case with Ruby and we can get away with these things.
Lets me take another concrete example. Lets say I need to find the row i such that square sum of elements in row i is the maximum. Now the solution of this is complicated in Python whereas with Ruby if correct functionality is in place just the following should work:
# Assume matrix is 2D matrix
res = matrix.map { |i| i**2 }.map_rows { |r| r.inject(:+) }.max_by { |i| i }To achieve the same with Python one needs to know about functions like sum, square, amin, and remember how they work but with Ruby you need to know about none.
It all came to when I went back from Ruby to NumPy because of a MOOC that requires it and I'm thinking how great things were in Ruby. I decided to write a blog post on this but it turns out that the NMatrix API is not like it should be so I can give examples of why I think Scientific Computation in Ruby is better.
While I can see an argument for changing this behavior, I'm concerned about the repercussions it will have on other elements of the API.
I strongly suggest changing this behavior and many others too if they don't blend well with the Ruby language. @zverok may be you could share your views on this.
Yes, you're preaching to the chorus about why to use Ruby instead of Python. But if you read through the commit history on these features, I think you'll find that we made these specific choices with a great deal of thought.
Ruby is simpler with each, map, etc. However, Ruby does not have n-dimensionally generalizable models for these functions, so we had to come up with our own.
If you'd like to write a pull request, you may certainly do so — but you'll need to make sure the new way of doing it is (1) efficient, (2) internally consistent, and (3) makes more sense in all cases than the old way.
@cjfuller Do you happen to recall this discussion? I feel like you were part of it at one point. I can't find it in my email.
Ruby is simpler with each, map, etc. However, Ruby does not have n-dimensionally generalizable models for these functions, so we had to come up with our own.
Yeah, I thought about that too. To support other dimensions we can use .each_axis(dim) so that each_axis(0) is equivalent to each_row and each_axis(1) is equivalent to each_col and so on.
That's what each_rank does. Please go read through enumerate.rb, as I already requested, before we continue this conversation. :)
We could add a pre-check in #max/#min/#mean to check if the NMatrix is a vector, and if it is, ignore whether it is a row or column vector and find the maximum along the long dimension by setting dimension to the greater shape value, thus not affecting how tensors are dealt with. I'm assuming finding the max along rows in a column vector and similar things is nonsensical.
Let's get the nomenclature clear here:
- a 1D vector is neither a row nor a column; it only has a shape of n.
- an n-D vector is a row or a column; its shape is typically [1,...,n], [n,...,1], or [1,...,n,...,1].
- an n-D tensor is anything where two or more components of the shape array are greater than 1.
Which do you mean?
If the input to #max is a 1D NMatrix (Shape = [n]) , or a 2D NMatrix Vector (Shape of [1,n] or [n,1]) , it is implicit in the first case that the dimension argument is 0 as handled correctly by the default, and binary in the second case (0 or 1, something we can check really easily and so won't have to require the user to specify). For all other shapes we need to specify the dimension as it's ambiguous.
I also notice that Matlab and most linear algebra libraries like to treat vectors as 2D arrays with one element of the shape being 1, so it would be better for #each_row and #each_column to return 2D NMatrix objects imo. (with respect to issue title)
It creates an inconsistency to treat 2D nmatrix vectors differently from n>2-D nmatrix vectors. Keep in mind that these vectors are an intermediate in many slicing operations.
I'm going to go ahead and close this issue. If you want to offer a patch which doesn't mess up 50% of the specs and makes sense, I'll entertain the change at that point.
Cool. I'll give it a try.