From 54458cb39d1081d0cfe6b77ed8e085d457a4c921 Mon Sep 17 00:00:00 2001 From: Rasmus Munk Larsen <4643818-rmlarsen1@users.noreply.gitlab.com> Date: Mon, 9 Mar 2026 00:35:26 -0700 Subject: [PATCH] Remove random retry loops in tests (batch 3: geometry, sparse, umeyama) libeigen/eigen!2262 Co-authored-by: Rasmus Munk Larsen --- test/geo_transformations.cpp | 20 ++++++------------ test/sparse_product.cpp | 8 ++++--- test/umeyama.cpp | 41 ++++-------------------------------- 3 files changed, 15 insertions(+), 54 deletions(-) diff --git a/test/geo_transformations.cpp b/test/geo_transformations.cpp index 68120d9d3..d10281a10 100644 --- a/test/geo_transformations.cpp +++ b/test/geo_transformations.cpp @@ -223,13 +223,10 @@ void transformations() { t4 *= aa3; VERIFY_IS_APPROX(t3.matrix(), t4.matrix()); - { - int guard = 0; - do { - v3 = Vector3::Random(); - dont_over_optimize(v3); - } while (v3.cwiseAbs().minCoeff() < NumTraits::epsilon() && (++guard) < 100); - VERIFY(guard < 100); + v3 = Vector3::Random(); + dont_over_optimize(v3); + for (int k = 0; k < 3; ++k) { + if (numext::abs(v3(k)) < NumTraits::epsilon()) v3(k) = NumTraits::epsilon(); } Translation3 tv3(v3); Transform3 t5(tv3); @@ -384,13 +381,8 @@ void transformations() { // test transform inversion t0.setIdentity(); t0.translate(v0); - { - int guard = 0; - do { - t0.linear().setRandom(); - } while (t0.linear().jacobiSvd().singularValues()(2) < test_precision() && (++guard) < 100); - VERIFY(guard < 100); - } + t0.linear().setRandom(); + if (t0.linear().jacobiSvd().singularValues()(2) < test_precision()) t0.linear().setIdentity(); Matrix4 t044 = Matrix4::Zero(); t044(3, 3) = 1; t044.block(0, 0, t0.matrix().rows(), 4) = t0.matrix(); diff --git a/test/sparse_product.cpp b/test/sparse_product.cpp index c006a8923..426a37557 100644 --- a/test/sparse_product.cpp +++ b/test/sparse_product.cpp @@ -301,9 +301,11 @@ void sparse_product() { SparseMatrixType mS(rows, rows); SparseMatrixType mA(rows, rows); initSparse(density, refA, mA); - do { - initSparse(density, refUp, mUp, ForceRealDiag | /*ForceNonZeroDiag|*/ MakeUpperTriangular); - } while (refUp.isZero()); + initSparse(density, refUp, mUp, ForceRealDiag | /*ForceNonZeroDiag|*/ MakeUpperTriangular); + if (refUp.isZero()) { + refUp(0, 0) = Scalar(1); + mUp.coeffRef(0, 0) = Scalar(1); + } refLo = refUp.adjoint(); mLo = mUp.adjoint(); refS = refUp + refLo; diff --git a/test/umeyama.cpp b/test/umeyama.cpp index 2f6fe34ec..e329f91aa 100644 --- a/test/umeyama.cpp +++ b/test/umeyama.cpp @@ -13,6 +13,7 @@ #include #include // required for MatrixBase::determinant +#include // required for HouseholderQR #include // required for SVD using namespace Eigen; @@ -23,43 +24,9 @@ Eigen::Matrix randMatrixUnitary(int size) { typedef T Scalar; typedef Eigen::Matrix MatrixType; - MatrixType Q; - - int max_tries = 40; - bool is_unitary = false; - - while (!is_unitary && max_tries > 0) { - // initialize random matrix - Q = MatrixType::Random(size, size); - - // orthogonalize columns using the Gram-Schmidt algorithm - for (int col = 0; col < size; ++col) { - typename MatrixType::ColXpr colVec = Q.col(col); - for (int prevCol = 0; prevCol < col; ++prevCol) { - typename MatrixType::ColXpr prevColVec = Q.col(prevCol); - colVec -= colVec.dot(prevColVec) * prevColVec; - } - Q.col(col) = colVec.normalized(); - } - - // this additional orthogonalization is not necessary in theory but should enhance - // the numerical orthogonality of the matrix - for (int row = 0; row < size; ++row) { - typename MatrixType::RowXpr rowVec = Q.row(row); - for (int prevRow = 0; prevRow < row; ++prevRow) { - typename MatrixType::RowXpr prevRowVec = Q.row(prevRow); - rowVec -= rowVec.dot(prevRowVec) * prevRowVec; - } - Q.row(row) = rowVec.normalized(); - } - - // final check - is_unitary = Q.isUnitary(); - --max_tries; - } - - if (max_tries == 0) eigen_assert(false && "randMatrixUnitary: Could not construct unitary matrix!"); - + // The Q factor of the QR decomposition of a random matrix is a random unitary matrix. + // HouseholderQR is numerically stable and always succeeds, unlike Gram-Schmidt. + MatrixType Q = MatrixType::Random(size, size).householderQr().householderQ(); return Q; }