From b8bb80400751b97a9ee8872efa7def6ebbb4b697 Mon Sep 17 00:00:00 2001
From: Benoit Jacob <jacob.benoit.1@gmail.com>
Date: Mon, 11 Oct 2010 21:00:42 -0400
Subject: [PATCH] set ColPivHouseholderQR as default preconditioner for
 JacobiSVD

---
 Eigen/src/Core/util/ForwardDeclarations.h |  2 +-
 Eigen/src/SVD/JacobiSVD.h                 | 16 ++++++++--------
 test/jacobisvd.cpp                        |  2 +-
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/Eigen/src/Core/util/ForwardDeclarations.h b/Eigen/src/Core/util/ForwardDeclarations.h
index e69957a31..1729432ca 100644
--- a/Eigen/src/Core/util/ForwardDeclarations.h
+++ b/Eigen/src/Core/util/ForwardDeclarations.h
@@ -170,7 +170,7 @@ template<typename MatrixType> class HouseholderQR;
 template<typename MatrixType> class ColPivHouseholderQR;
 template<typename MatrixType> class FullPivHouseholderQR;
 template<typename MatrixType> class SVD;
-template<typename MatrixType, int QRPreconditioner = FullPivHouseholderQRPreconditioner> class JacobiSVD;
+template<typename MatrixType, int QRPreconditioner = ColPivHouseholderQRPreconditioner> class JacobiSVD;
 template<typename MatrixType, int UpLo = Lower> class LLT;
 template<typename MatrixType, int UpLo = Lower> class LDLT;
 template<typename VectorsType, typename CoeffsType, int Side=OnTheLeft> class HouseholderSequence;
diff --git a/Eigen/src/SVD/JacobiSVD.h b/Eigen/src/SVD/JacobiSVD.h
index a4c2fb91d..e7156a3fb 100644
--- a/Eigen/src/SVD/JacobiSVD.h
+++ b/Eigen/src/SVD/JacobiSVD.h
@@ -205,17 +205,17 @@ struct ei_qr_preconditioner_impl<MatrixType, HouseholderQRPreconditioner, Precon
   *                        for the R-SVD step for non-square matrices. See discussion of possible values below.
   *
   * The possible values for QRPreconditioner are:
-  * \li FullPivHouseholderQRPreconditioner (the default), is the safest and slowest. It uses full-pivoting QR.
-  *     We make it the default so that JacobiSVD is guaranteed to be entirely, uncompromisingly safe by default.
+  * \li ColPivHouseholderQRPreconditioner is the default. In practice it's very safe. It uses column-pivoting QR.
+  * \li FullPivHouseholderQRPreconditioner, is the safest and slowest. It uses full-pivoting QR.
   *     Contrary to other QRs, it doesn't allow computing thin unitaries.
-  * \li ColPivHouseholderQRPreconditioner is faster, and in practice still very safe, although theoretically not as safe as the default
-  *     full-pivoting preconditioner. It uses column-pivoting QR.
-  * \li HouseholderQRPreconditioner is even faster, and less safe and accurate than the pivoting variants. It uses non-pivoting QR.
+  * \li HouseholderQRPreconditioner is the fastest, and less safe and accurate than the pivoting variants. It uses non-pivoting QR.
   *     This is very similar in safety and accuracy to the bidiagonalization process used by bidiagonalizing SVD algorithms (since bidiagonalization
-  *     is inherently non-pivoting).
-  * \li NoQRPreconditioner allows to not use a QR preconditioner at all. This is useful if you know that you will only be computing
+  *     is inherently non-pivoting). However the resulting SVD is still more reliable than bidiagonalizing SVDs because the Jacobi-based iterarive
+  *     process is more reliable than the optimized bidiagonal SVD iterations.
+  * \li NoQRPreconditioner allows not to use a QR preconditioner at all. This is useful if you know that you will only be computing
   *     JacobiSVD decompositions of square matrices. Non-square matrices require a QR preconditioner. Using this option will result in
-  *     faster compilation and smaller executable code.
+  *     faster compilation and smaller executable code. It won't significantly speed up computation, since JacobiSVD is always checking
+  *     if QR preconditioning is needed before applying it anyway.
   *
   * \sa MatrixBase::jacobiSvd()
   */
diff --git a/test/jacobisvd.cpp b/test/jacobisvd.cpp
index 935d0ac05..0039be388 100644
--- a/test/jacobisvd.cpp
+++ b/test/jacobisvd.cpp
@@ -159,7 +159,7 @@ template<typename MatrixType> void jacobisvd_verify_assert(const MatrixType& m)
 
   RhsType rhs(rows);
 
-  JacobiSVD<MatrixType, HouseholderQRPreconditioner> svd;
+  JacobiSVD<MatrixType> svd;
   VERIFY_RAISES_ASSERT(svd.matrixU())
   VERIFY_RAISES_ASSERT(svd.singularValues())
   VERIFY_RAISES_ASSERT(svd.matrixV())