Fix QRFactor to be QR, use lapack calls, and add Solve capacity.

[btracey]

No description provided.

[btracey]

I have disabled the LQ test for now because there is temporarily a number of inconsistencies between it and the new QR implementation. Additionally, it does not test the correct behavior. It should test that the normal equations hold, not merely that A* x = b.

LQ test will be fixed in the next commit when LQ is modified.

[btracey]

This needs gonum/lapack#33

[btracey]

Rebased onto master.

[btracey]

Alternately, qr could be disguised as a TriDense and then use Dense.Copy

[kortschak]

I like it, but it's sneaky (maybe this is why I like it). If you do this, hide the sneakiness in a function called asTriDense(*QR) *TriDense. Then at least we are being open about it.

[btracey]

It's not obvious when the TriDense is being constructed right there? seeing

....
Stride: qr.qr.mat.Stride
Data: qr.qr.mat.Data

seems pretty obviously sneaky (but then again maybe I've been living in lapack too much)

[kortschak]

but then again maybe I've been living in lapack too much

Maybe.

If you have a call m.Copy(asTriDense(qr)) it's pretty obvious what is happening. Though we should add the triangular cases to Dense.Copy since they currently get handled as the slow path.

[btracey]

So let's leave this for now and we can modify when the fast path is in.
On Aug 12, 2015 6:50 PM, "Dan Kortschak" notifications@github.com wrote:

In mat64/qr.go
#184 (comment):

}

-// IsFullRank returns whether the R matrix and hence a has full rank.
-func (f QRFactor) IsFullRank() bool {

• for _, v := range f.rDiag {

•   if v == 0 {
  

•       return false
  

  +// TODO(btracey): Add in the "Reduced" forms for extracting the n×n orthogonal
  +// and upper triangular matrices.
  +
  +// RFromQR extracts the m×n upper trapezoidal matrix from a QR decomposition.
  +func (m *Dense) RFromQR(qr *QR) {
• r, c := qr.qr.Dims()
• m.reuseAs(r, c)
• for i := 0; i < r; i++ {

•   for j := 0; j < min(i, c); j++ {
  

but then again maybe I've been living in lapack too much

Maybe.

If you have a call m.Copy(asTriDense(qr)) it's pretty obvious what is
happening. Though we should add the triangular cases to Dense.Copy since
they currently get handled as the slow path.

—
Reply to this email directly or view it on GitHub
https://github.com/gonum/matrix/pull/184/files#r36931895.

[btracey]

Would you like me to update this with the new Copy code? Anything else?

[kortschak]

Would you like me to update this with the new Copy code? Anything else?

Which new Copy code? The TriDense.Copy isn't relevant here is it? The discussion here relates to extraction to a Dense. That should just be an unexported helper here.

Everything else seems fine.