DenseMatrix64F.reshape() gives wrong result when saveValues is true

[GoogleCodeExporter]

What steps will reproduce the problem?
1. consider the following matrix:

1 0
0 1

2. reshape it by adding a new column:
DenseMatrix64F.reshape(2, 3, true)

What is the expected output? What do you see instead?
Expected:
1 0 0
0 1 0

What I see:
1 0 0
1 0 0

What version of the product are you using? On what operating system?
Using the 0.17 version on Windows 7.

Please provide any additional information below.
When copying the original matrix, the internal array is simply copied into the 
new array, which is larger by the necessary amount of elements. This way the 
new elements get positioned to the END of the internal array, not to every 
numCol()th position.

Original issue reported on code.google.com by sh10...@freemail.hu on 22 Nov 2011 at 7:45

[GoogleCodeExporter]

Attached patch file.

Original comment by sh10...@freemail.hu on 22 Nov 2011 at 9:08

Attachments:

• DenseMatrix64F.java.patch

[GoogleCodeExporter]

The documentation for this function was confusing, but its behavior appears to 
be correct.  The current behavior is similar to how matlab/octave implement 
reshape.  However in EJML values are shuffled using a row-major ordering 
compared to a column-major ordering in Matlab.

See new Java doc below and let me know if that clarifies the problem.  Thanks 
for the bug report and taking the time to prepare a patch.  Looking at the 
insert code, its probably a bit slow and could be speed up doing something 
similar to your patch.

What you want to do is grow the matrix.  The easiest way to do that is to 
declare a new matrix and insert the old one into that:

        DenseMatrix64F A = new DenseMatrix64F(2,2,true,1,2,3,4);
        DenseMatrix64F B = new DenseMatrix64F(3,2);

        CommonOps.insert(A,B,0,0);

At one point I considered adding a grow()/shrink() function but thought that 
might encourage bad coding habits.  Do you think that should be reconsidered?

-------------- New JavaDoc below ----------------
    /**
     * <p>
     * Changes the number of rows and columns in the matrix, allowing its size to grow or shrink.
     * If the saveValues flag is set to true, then the previous values will be maintained, but
     * reassigned to new elements in a row-major ordering.  If saveValues is false values will only
     * be maintained when the requested size is less than or equal to the internal array size.
     * The primary use for this function is to encourage data reuse and avoid unnecessarily declaring
     * and initialization of new memory.
     * </p>
     * 
     * <p>
     * Examples:<br>
     * [ 1 2 ; 3 4 ] -> reshape( 2 , 3 , true ) = [ 1 2 3 ; 4 0 0 ]<br>
     * [ 1 2 ; 3 4 ] -> reshape( 1 , 2 , true ) = [ 1 2 ]<br>
     * [ 1 2 ; 3 4 ] -> reshape( 1 , 2 , false ) = [ 1 2 ]<br>
     * [ 1 2 ; 3 4 ] -> reshape( 2 , 3 , false ) = [ 0 0 0 ; 0 0 0 ]
     * </p>
     *
     * @param numRows The new number of rows in the matrix.
     * @param numCols The new number of columns in the matrix.
     * @param saveValues If true then the value of each element will be save using a row-major reordering.  Typically this should be false.
     */

Original comment by peter.ab...@gmail.com on 23 Nov 2011 at 12:15

[GoogleCodeExporter]

The new documentation is very clear.

I've never used Matlab or Octave. Are there use cases which justify the current 
behavior? I couldn't find any, where the seemingly shuffled values would be 
useful, but my experience with matrices is somewhat limited.

I think I see why grow()/shrink() would encourage bad coding habits, thinking 
over what I use them for currently. I think you cannot prevent bad coding 
habits this way: without your comment, I would just write my own grow() method 
which encapsulate the code snippet you wrote as the easiest way to grow a 
matrix.

Original comment by sh10...@freemail.hu on 23 Nov 2011 at 6:56

[GoogleCodeExporter]

Because Matlab is so popular its behavior, for better or worse, tends to be a 
de facto standard.  The main reason grow()/shrink() is bad is because people 
tend to use grow() inside of loops, causing a massive amount of memory copying. 
In EJML a better approach would be to declare a large matrix, then use resize 
to make it smaller.  If you absolutely must "grow" as rows are added you could 
then resize it. That strategy requires no memory copying.  If it's growing 
column wise, simply grow it along the rows instead and transpose the matrix at 
the end.

That also answers your question about a use case for the resize().

Unless you have any objections, I'm going to close this ticket soon.

Original comment by peter.ab...@gmail.com on 24 Nov 2011 at 5:53

[GoogleCodeExporter]

I have no objections against closing the issue. Thank you very much for your 
answers!

Original comment by sh10...@freemail.hu on 24 Nov 2011 at 6:56

[GoogleCodeExporter]

Original comment by peter.ab...@gmail.com on 24 Nov 2011 at 11:06

• Changed state: Done