PR replies

gonum · Jan 5, 2016 · 46df40b · 46df40b
1 parent 69d6a03
commit 46df40b
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 14 deletions.
diff --git a/cgo/lapack.go b/cgo/lapack.go
@@ -498,6 +498,8 @@ func (impl Implementation) Dgels(trans blas.Transpose, m, n, nrhs int, a []float
 	return clapack.Dgels(trans, m, n, nrhs, a, lda, b, ldb)
 }
 
+const noSVDO = "dgesvd: not coded for overwrite"
+
 // Dgesvd computes the singular value decomposition of the input matrix A.
 //
 // The singular value decomposition is
@@ -514,7 +516,7 @@ func (impl Implementation) Dgels(trans blas.Transpose, m, n, nrhs int, a []float
 //  jobU == lapack.SVDOverwrite The first min(m,n) columns of U are written into a
 //  jobU == lapack.SVDNone      The columns of U are not computed.
 // The behavior is the same for jobVT and the rows of V^T. At most one of jobU
-// and jobVT can equal lapack.SVDOverwrite.
+// and jobVT can equal lapack.SVDOverwrite, and Dgesvd will panic otherwise.
 //
 // On entry, a contains the data for the m×n matrix A. During the call to Dgesvd
 // the data is overwritten. On exit, A contains the appropriate singular vectors
@@ -529,12 +531,12 @@ func (impl Implementation) Dgels(trans blas.Transpose, m, n, nrhs int, a []float
 // not used.
 //
 // vt contains the left singular vectors on exit, stored rowwise. If
-// jobV == lapack.SVDAll, vt is of size n×m. If jobV == lapack.SVDInPlace vt is
-// of size min(m,n)×n. If jobU == lapack.SVDOverwrite or lapack.SVDNone, vt is
+// jobV == lapack.SVDAll, vt is of size n×m. If jobVT == lapack.SVDInPlace vt is
+// of size min(m,n)×n. If jobVT == lapack.SVDOverwrite or lapack.SVDNone, vt is
 // not used.
 //
 // The C interface does not support providing temporary storage. To provide compatibility
-// with native, lwork == -1 will not run Dgeqrf but will instead write the minimum
+// with native, lwork == -1 will not run Dgesvd but will instead write the minimum
 // work necessary to work[0]. If len(work) < lwork, Dgeqrf will panic.
 //
 // Dgesvd returns whether the decomposition successfully completed.
@@ -551,7 +553,7 @@ func (impl Implementation) Dgesvd(jobU, jobVT lapack.SVDJob, m, n int, a []float
 		checkMatrix(min(m, n), n, vt, ldvt)
 	}
 	if jobU == lapack.SVDOverwrite && jobVT == lapack.SVDOverwrite {
-		panic("lapack: both jobU and jobVT are lapack.SVDOverwrite")
+		panic(noSVDO)
 	}
 	if len(s) < min(m, n) {
 		panic(badS)
@@ -572,7 +574,7 @@ func (impl Implementation) Dgesvd(jobU, jobVT lapack.SVDJob, m, n int, a []float
 		work[0] = float64(minWork)
 		return true
 	}
-	return clapack.Dgesvd(lapack.Job(jobU), lapack.Job(jobVT), m, n, a, lda, s, u, ldu, vt, ldvt, work)
+	return clapack.Dgesvd(lapack.Job(jobU), lapack.Job(jobVT), m, n, a, lda, s, u, ldu, vt, ldvt, work[1:])
 }
 
 // Dgetf2 computes the LU decomposition of the m×n matrix A.

diff --git a/native/dgesvd.go b/native/dgesvd.go
@@ -16,8 +16,6 @@ const noSVDO = "dgesvd: not coded for overwrite"
 
 // Dgesvd computes the singular value decomposition of the input matrix A.
 //
-// Only coded for jobU == lapack.SVDAll and jobVT == lapack.SVDAll.
-//
 // The singular value decomposition is
 //  A = U * Sigma * V^T
 // where Sigma is an m×n diagonal matrix containing the singular values of A,
@@ -27,12 +25,12 @@ const noSVDO = "dgesvd: not coded for overwrite"
 //
 // jobU and jobVT are options for computing the singular vectors. The behavior
 // is as follows
-//  jobU == lapack.SVDAll       All m columns of U are returned in u
+//  jobU == lapack.SVDAll       All M columns of U are returned in u
 //  jobU == lapack.SVDInPlace   The first min(m,n) columns are returned in u
 //  jobU == lapack.SVDOverwrite The first min(m,n) columns of U are written into a
 //  jobU == lapack.SVDNone      The columns of U are not computed.
 // The behavior is the same for jobVT and the rows of V^T. At most one of jobU
-// and jobVT can equal lapack.SVDOverwrite.
+// and jobVT can equal lapack.SVDOverwrite, and Dgesvd will panic otherwise.
 //
 // On entry, a contains the data for the m×n matrix A. During the call to Dgesvd
 // the data is overwritten. On exit, A contains the appropriate singular vectors
@@ -47,8 +45,8 @@ const noSVDO = "dgesvd: not coded for overwrite"
 // not used.
 //
 // vt contains the left singular vectors on exit, stored rowwise. If
-// jobV == lapack.SVDAll, vt is of size n×m. If jobV == lapack.SVDInPlace vt is
-// of size min(m,n)×n. If jobU == lapack.SVDOverwrite or lapack.SVDNone, vt is
+// jobV == lapack.SVDAll, vt is of size n×m. If jobVT == lapack.SVDInPlace vt is
+// of size min(m,n)×n. If jobVT == lapack.SVDOverwrite or lapack.SVDNone, vt is
 // not used.
 //
 // work is a slice for storing temporary memory, and lwork is the usable size of
@@ -77,7 +75,7 @@ func (impl Implementation) Dgesvd(jobU, jobVT lapack.SVDJob, m, n int, a []float
 		panic(badS)
 	}
 	if jobU == lapack.SVDOverwrite || jobVT == lapack.SVDOverwrite {
-		panic("lapack: SVD not coded to overwrite original matrix")
+		panic(noSVDO)
 	}
 	minWork := max(5*min(m, n), 3*min(m, n)+max(m, n))
 	if lwork != -1 {
@@ -518,7 +516,6 @@ func (impl Implementation) Dgesvd(jobU, jobVT lapack.SVDJob, m, n int, a []float
 						impl.Dlacpy(blas.Upper, n, n, work[iu:], ldworku, vt, ldvt)
 
 						// Generate left bidiagonalizing vectors in work[iu]
-
 						impl.Dorgbr(lapack.ApplyQ, n, n, n, work[iu:], ldworku, work[itauq:], work[iwork:], lwork-iwork)
 
 						// Generate right bidiagonalizing vectors in VT