From c0c8972d0df0f52c9ac95a89fe2a7b1e9a921b0e Mon Sep 17 00:00:00 2001 From: Frank Dellaert Date: Wed, 19 Feb 2025 23:38:43 -0500 Subject: [PATCH] inplace_QR *already* zeros out below diagonal --- gtsam/linear/JacobianFactor.cpp | 2 -- gtsam/linear/NoiseModel.cpp | 9 --------- 2 files changed, 11 deletions(-) diff --git a/gtsam/linear/JacobianFactor.cpp b/gtsam/linear/JacobianFactor.cpp index 1802475eb..82d0eec28 100644 --- a/gtsam/linear/JacobianFactor.cpp +++ b/gtsam/linear/JacobianFactor.cpp @@ -827,8 +827,6 @@ std::pair Eliminate // The inplace variant will have no valid rows anymore below m==n // and only entries above the diagonal are valid. inplace_QR(Ab.matrix()); - // We zero below the diagonal to agree with the result from noieModel QR - Ab.matrix().triangularView().setZero(); size_t m = Ab.rows(), n = Ab.cols() - 1; size_t maxRank = min(m, n); jointFactor->model_ = noiseModel::Unit::Create(maxRank); diff --git a/gtsam/linear/NoiseModel.cpp b/gtsam/linear/NoiseModel.cpp index 2f693bbc4..865e04c1f 100644 --- a/gtsam/linear/NoiseModel.cpp +++ b/gtsam/linear/NoiseModel.cpp @@ -191,8 +191,6 @@ SharedDiagonal Gaussian::QR(Matrix& Ab) const { gttic(Gaussian_noise_model_QR); - static const bool debug = false; - // get size(A) and maxRank // TODO: really no rank problems ? size_t m = Ab.rows(), n = Ab.cols()-1; @@ -201,15 +199,8 @@ SharedDiagonal Gaussian::QR(Matrix& Ab) const { // pre-whiten everything (cheaply if possible) WhitenInPlace(Ab); - if(debug) gtsam::print(Ab, "Whitened Ab: "); - // Eigen QR - much faster than older householder approach inplace_QR(Ab); - Ab.triangularView().setZero(); - - // hand-coded householder implementation - // TODO: necessary to isolate last column? - // householder(Ab, maxRank); return noiseModel::Unit::Create(maxRank); }