Wrong size check at fullPivHouseholderQRSolve?

[degill]

Hey there,

I have a question about DenseMatrix::1034:

it checks if

if( this.cols != b.rows )
    throw new ...

thus enforcing that

this.cols == b.rows

which is, unless I am making a fool out of myself, the wrong condition to ask for. It rather should be

if (this.rows != b.rows)

A matrix multiplication is defined as a mapping from

this^(m,n) X result^(n,k) --> b^(m,k)

As an example

this: (a,b,c) = this^(1,3); result: (x,y,z)^t = b^(3,1) this X result = b = ax+by+cz = dimension of (1,1)

Thus, it is important that this.rows equals b.rows. this.cols can certainly be different from b.rows (see example)

The code goes on to allocate a DenseMatrix of the correct format

new DenseMatrix(this.cols, b.cols)

result^(n,k)

Am I wrong?

[hughperkins]

Hmmmm, seems convincing. The test is using a square matrix and so doesnt pick this up :-P Taking a look...

[degill]

Yes, I had a look at the test and saw that i was not a comprehensive test ;) the if condition could not really apply for that

[hughperkins]

Hmmm, segfaults. Thats puzzling...

[hughperkins]

Try now? Note that you might need to rm -Rf ~/.jeigen after rebuilding (or Windows equivalent). Changes:

• upgraded to Eigen 3.2.5
• fixed the if as per your suggestion
• modified the test to use non-square matrix