DenseMatrix64F.reshape() gives wrong result when saveValues is true about efficient-java-matrix-library HOT 6 CLOSED

Attached patch file.

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.
     */

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.

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.

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