Project

General

Profile

Actions

Feature #10077

closed

[PATCH] Implement Matrix#row_merge and Matrix#column_merge

Added by gogotanaka (Kazuki Tanaka) over 9 years ago. Updated over 9 years ago.

Status:
Closed
Target version:
-
[ruby-core:63910]

Description

Merge matrices horizontally and vertically.

It is useful and important when we handle linear equations, statistics and so on.

Matrix[[1, 2], [3, 4]].row_merge(Matrix[[5], [6]])

  => 1 2 5
     3 4 6

Matrix[[1, 2], [3, 4]].column_merge(Matrix[[5, 6]])

  => 1 2
     3 4
     5 6

# accept more than two matrices as an argument.

Matrix[[1, 2], [3, 4]].column_merge(Matrix[[5, 6]], Matrix[[7, 8]])

  => 1 2
     3 4
     5 6
     7 8
  • Matrix#row_merge needs Matrix#column

Files

Implement_Matrix#column_merge.patch (863 Bytes) Implement_Matrix#column_merge.patch gogotanaka (Kazuki Tanaka), 07/21/2014 02:51 AM
Add_test_for_Matrix#column_merge.patch (1.36 KB) Add_test_for_Matrix#column_merge.patch gogotanaka (Kazuki Tanaka), 07/21/2014 02:51 AM
Add_Matrix#column_merge_to_NEWS.patch (452 Bytes) Add_Matrix#column_merge_to_NEWS.patch gogotanaka (Kazuki Tanaka), 07/21/2014 02:51 AM
Implement_Matrix#row_merge.patch (899 Bytes) Implement_Matrix#row_merge.patch gogotanaka (Kazuki Tanaka), 07/21/2014 02:52 AM
Add_test_for_Matrix#row_merge.patch (1.4 KB) Add_test_for_Matrix#row_merge.patch gogotanaka (Kazuki Tanaka), 07/21/2014 02:52 AM
Add_Matrix#row_merge_to_NEWS.patch (523 Bytes) Add_Matrix#row_merge_to_NEWS.patch gogotanaka (Kazuki Tanaka), 07/21/2014 02:52 AM

Updated by gogotanaka (Kazuki Tanaka) over 9 years ago

Is there anything I should do with?

Updated by hsbt (Hiroshi SHIBATA) over 9 years ago

  • Status changed from Open to Assigned
  • Assignee set to marcandre (Marc-Andre Lafortune)

Updated by gogotanaka (Kazuki Tanaka) over 9 years ago

@hiroshi (Hiroshi MORIYAMA) SHIBATA san

Thank you so much!

いつもありがとうございます. 助かっています.

Updated by david_macmahon (David MacMahon) over 9 years ago

FWIW, Matlab calls these operations "horzcat" (for "horizontal concatenation") and "vertcat" (for "vertical concatenation"). I personally find these names less ambiguous than "row_merge" and "column_merge". IMHO, the name "row_merge" seems to have an ambiguity as to whether it will merge along the rows of the Matrix (creating more columns) or whether it will merge additional rows onto the Matrix, but maybe that's just my lack of familiarity with these new names.

On Sep 12, 2014, at 10:11 AM, wrote:

Issue #10077 has been updated by gogo tanaka.

@hiroshi (Hiroshi MORIYAMA) SHIBATA san

Thank you so much!

いつもありがとうございます. 助かっています.


Feature #10077: [PATCH] Implement Matrix#row_merge and Matrix#column_merge
https://bugs.ruby-lang.org/issues/10077#change-48878

  • Author: gogo tanaka
  • Status: Assigned
  • Priority: Normal
  • Assignee: Marc-Andre Lafortune
  • Category:
  • Target version:

Merge matrices horizontally and vertically.

It is useful and important when we handle linear equations, statistics and so on.

Matrix[[1, 2], [3, 4]].row_merge(Matrix[[5], [6]])

 => 1 2 5
    3 4 6

Matrix[[1, 2], [3, 4]].column_merge(Matrix[[5, 6]])

 => 1 2
    3 4
    5 6

# accept more than two matrices as an argument.

Matrix[[1, 2], [3, 4]].column_merge(Matrix[[5, 6]], Matrix[[7, 8]])

 => 1 2
    3 4
    5 6
    7 8
  • Matrix#row_merge needs Matrix#column

---Files--------------------------------
Implement_Matrix#column_merge.patch (863 Bytes)
Add_test_for_Matrix#column_merge.patch (1.36 KB)
Add_Matrix#column_merge_to_NEWS.patch (452 Bytes)
Implement_Matrix#row_merge.patch (899 Bytes)
Add_test_for_Matrix#row_merge.patch (1.4 KB)
Add_Matrix#row_merge_to_NEWS.patch (523 Bytes)

--
https://bugs.ruby-lang.org/

Updated by gogotanaka (Kazuki Tanaka) over 9 years ago

@david (david he) MacMahon

Thank you!

It make sense for me.

As you said,

the name "row_merge" seems to have an ambiguity as to whether it will merge along the rows of the Matrix (creating more columns) or whether it will merge additional rows onto the Matrix

I agree with this, but horzcat or vertcat are hard to understand for people who isn't familiar with Matlab.

So I'm gonna propose horz_merge and vert_merge. Dose it make sense?

Updated by david_macmahon (David MacMahon) over 9 years ago

On Sep 16, 2014, at 4:21 PM, wrote:

So I propose horz_merge and vert_merge. Dose it make sense?

I definitely like it better than row_merge and col_merge, but to me the verb "merge" has connotations of intertwining or melding. IMHO, "merge" makes sense for Hash or Set, but not so much for Matrix. Since we already have Array#concat, how about Matrix#horz_concat and Matrix#vert_concat? Just an idea...

Updated by marcandre (Marc-Andre Lafortune) over 9 years ago

  • Category set to lib
  • Status changed from Assigned to Closed

I've used hstack and vstack, as suggested in https://github.com/ruby/ruby/pull/344

I hope this satisfied everyone :-)

Committed as r47769

Updated by gogotanaka (Kazuki Tanaka) over 9 years ago

Sure, no problem.

Updated by gogotanaka (Kazuki Tanaka) over 9 years ago

@Marc-Andre Lafortune san

It’s hard for me to say this, but I have confidence in my implementation more than merged one.

My implementation

def row_merge(*matrices)
  if matrices.any?{ |m| m.row_size != row_size }
    raise ErrDimensionMismatch, "all matrices should have same row size"
  end
  transpose.column_merge(*matrices.map(&:transpose)).transpose
end
alias_method :merge, :row_merge

def column_merge(*matrices)
  if matrices.any?{ |m| m.column_size != column_size }
    raise ErrDimensionMismatch, "all matrices should have same column size"
  end
  new_matrix [self, *matrices].map(&:to_a).flatten(1)
end

Merged implementation

def Matrix.hstack(x, *matrices)
  raise TypeError, "Expected a Matrix, got a #{x.class}" unless x.is_a?(Matrix)
  result = x.send(:rows).map(&:dup)
  total_column_count = x.column_count
  matrices.each do |m|
    raise TypeError, "Expected a Matrix, got a #{m.class}" unless m.is_a?(Matrix)
    if m.row_count != x.row_count
      raise ErrDimensionMismatch, "The given matrices must have #{x.row_count} rows, but one has #{m.row_count}"
    end
    result.each_with_index do |row, i|
      row.concat m.send(:rows)[i]
    end
    total_column_count += m.column_count
  end
  new result, total_column_count
end



def Matrix.vstack(x, *matrices)
  raise TypeError, "Expected a Matrix, got a #{x.class}" unless x.is_a?(Matrix)
  result = x.send(:rows).map(&:dup)
  matrices.each do |m|
    raise TypeError, "Expected a Matrix, got a #{m.class}" unless m.is_a?(Matrix)
    if m.column_count != x.column_count
      raise ErrDimensionMismatch, "The given matrices must have #{x.column_count} columns, but one has #{m.column_count}"
    end
    result.concat(m.send(:rows))
  end
  new result, x.column_count
end

I believe the interface x.row_merge(y) is preferred to Matrix.vstack(x, y),

And I think my implementation is more clear and maintainability.

Of course, merged one has some better point my patch doesn't have.

But we should use symmetry between row and column at least.

How is merged implementation preferred to my implementation?

I'm so sorry, I said something trouble.

Take you time.

Updated by marcandre (Marc-Andre Lafortune) over 9 years ago

Thank you for your comments.

I want to point out that I've added both x.hstack(y) and Matrix.hstack(x, y), so both interfaces are available now, the same way I would love for Array#product to have a corresponding Array.product (see https://bugs.ruby-lang.org/issues/8970)

Your proposed implementation using transpose is shorter, clever and expressive. It's very nice. I've chosen mine for efficiency:

require 'fruity'
m = Matrix.identity(3)
compare do
  row_merge { m.row_merge(m) }
  hstack     { m.hstack(m) }
end
# => hstack is faster than row_merge by 2x ± 0.1

For bigger matrices, the performance gap gets bigger (8x for 100x100 and 23x for 1000x1000).

Updated by gogotanaka (Kazuki Tanaka) over 9 years ago

@Marc-Andre Lafortune

Thank you for the thorough explanation.

OK, I understand what you said, this satisfied me now.

I realize we'd better think about efficiency more than maintainability or brevity of codes.
(There might be some exceptions)

Thanks.

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0