From a0ae31f0d475e5aa7e5d5c853fcbc56e229a6f90 Mon Sep 17 00:00:00 2001 From: Tyler Veness Date: Sun, 29 Dec 2024 11:57:28 -0800 Subject: [PATCH 1/3] Remove operator^ from Matrix and SubgraphPreconditioner Eigen recently added an operator^ for coefficient-wise bitwise xor, which conflicts with gtsam's operator^. Since gtsam's operator^ isn't xor, I opted to remove it. Fixes #1951. --- gtsam/base/Matrix.cpp | 11 ----------- gtsam/base/Matrix.h | 6 ------ gtsam/linear/SubgraphPreconditioner.cpp | 14 -------------- gtsam/linear/SubgraphPreconditioner.h | 3 --- gtsam/linear/iterative.h | 7 +------ 5 files changed, 1 insertion(+), 40 deletions(-) diff --git a/gtsam/base/Matrix.cpp b/gtsam/base/Matrix.cpp index 1e04bc58a8..6248ad77de 100644 --- a/gtsam/base/Matrix.cpp +++ b/gtsam/base/Matrix.cpp @@ -126,17 +126,6 @@ bool linear_dependent(const Matrix& A, const Matrix& B, double tol) { } /* ************************************************************************* */ -Vector operator^(const Matrix& A, const Vector & v) { - if (A.rows()!=v.size()) { - throw std::invalid_argument("Matrix operator^ : A.m(" + std::to_string(A.rows()) + ")!=v.size(" + - std::to_string(v.size()) + ")"); - } -// Vector vt = v.transpose(); -// Vector vtA = vt * A; -// return vtA.transpose(); - return A.transpose() * v; -} - const Eigen::IOFormat& matlabFormat() { static const Eigen::IOFormat matlab( Eigen::StreamPrecision, // precision diff --git a/gtsam/base/Matrix.h b/gtsam/base/Matrix.h index c8dc46ed51..2b177e3d73 100644 --- a/gtsam/base/Matrix.h +++ b/gtsam/base/Matrix.h @@ -132,12 +132,6 @@ GTSAM_EXPORT bool linear_independent(const Matrix& A, const Matrix& B, double to */ GTSAM_EXPORT bool linear_dependent(const Matrix& A, const Matrix& B, double tol = 1e-9); -/** - * overload ^ for trans(A)*v - * We transpose the vectors for speed. - */ -GTSAM_EXPORT Vector operator^(const Matrix& A, const Vector & v); - /** products using old-style format to improve compatibility */ template inline MATRIX prod(const MATRIX& A, const MATRIX&B) { diff --git a/gtsam/linear/SubgraphPreconditioner.cpp b/gtsam/linear/SubgraphPreconditioner.cpp index 53ea94d6eb..9087b3c909 100644 --- a/gtsam/linear/SubgraphPreconditioner.cpp +++ b/gtsam/linear/SubgraphPreconditioner.cpp @@ -149,20 +149,6 @@ void SubgraphPreconditioner::multiplyInPlace(const VectorValues& y, Errors& e) c Ab2_.multiplyInPlace(x, ei); // use iterator version } -/* ************************************************************************* */ -// Apply operator A', A'*e = [I inv(R1')*A2']*e = e1 + inv(R1')*A2'*e2 -VectorValues SubgraphPreconditioner::operator^(const Errors& e) const { - - Errors::const_iterator it = e.begin(); - VectorValues y = zero(); - for(auto& key_value: y) { - key_value.second = *it; - ++it; - } - transposeMultiplyAdd2(1.0, it, e.end(), y); - return y; -} - /* ************************************************************************* */ // y += alpha*A'*e void SubgraphPreconditioner::transposeMultiplyAdd diff --git a/gtsam/linear/SubgraphPreconditioner.h b/gtsam/linear/SubgraphPreconditioner.h index 558a80bd86..9e005dd53d 100644 --- a/gtsam/linear/SubgraphPreconditioner.h +++ b/gtsam/linear/SubgraphPreconditioner.h @@ -125,9 +125,6 @@ namespace gtsam { /** Apply operator A in place: needs e allocated already */ void multiplyInPlace(const VectorValues& y, Errors& e) const; - /** Apply operator A' */ - VectorValues operator^(const Errors& e) const; - /** * Add A'*e to y * y += alpha*A'*[e1;e2] = [alpha*e1; alpha*inv(R1')*A2'*e2] diff --git a/gtsam/linear/iterative.h b/gtsam/linear/iterative.h index 22f65b8de8..ff3d738019 100644 --- a/gtsam/linear/iterative.h +++ b/gtsam/linear/iterative.h @@ -59,11 +59,6 @@ namespace gtsam { /** Access b vector */ const Vector& b() const { return b_; } - /** Apply operator A'*e */ - Vector operator^(const Vector& e) const { - return A_ ^ e; - } - /** * Print with optional string */ @@ -71,7 +66,7 @@ namespace gtsam { /** gradient of objective function 0.5*|Ax-b_|^2 at x = A_'*(Ax-b_) */ Vector gradient(const Vector& x) const { - return A() ^ (A() * x - b()); + return A().transpose() * (A() * x - b()); } /** Apply operator A */ From 5a44974785776bd18442c0bb38362490d2a62cfc Mon Sep 17 00:00:00 2001 From: Tyler Veness Date: Fri, 10 Jan 2025 15:36:09 -0800 Subject: [PATCH 2/3] Reverted interface changes that are unnecessary to make the code compile --- gtsam/linear/SubgraphPreconditioner.cpp | 14 ++++++++++++++ gtsam/linear/SubgraphPreconditioner.h | 3 +++ gtsam/linear/iterative.h | 5 +++++ 3 files changed, 22 insertions(+) diff --git a/gtsam/linear/SubgraphPreconditioner.cpp b/gtsam/linear/SubgraphPreconditioner.cpp index 9087b3c909..53ea94d6eb 100644 --- a/gtsam/linear/SubgraphPreconditioner.cpp +++ b/gtsam/linear/SubgraphPreconditioner.cpp @@ -149,6 +149,20 @@ void SubgraphPreconditioner::multiplyInPlace(const VectorValues& y, Errors& e) c Ab2_.multiplyInPlace(x, ei); // use iterator version } +/* ************************************************************************* */ +// Apply operator A', A'*e = [I inv(R1')*A2']*e = e1 + inv(R1')*A2'*e2 +VectorValues SubgraphPreconditioner::operator^(const Errors& e) const { + + Errors::const_iterator it = e.begin(); + VectorValues y = zero(); + for(auto& key_value: y) { + key_value.second = *it; + ++it; + } + transposeMultiplyAdd2(1.0, it, e.end(), y); + return y; +} + /* ************************************************************************* */ // y += alpha*A'*e void SubgraphPreconditioner::transposeMultiplyAdd diff --git a/gtsam/linear/SubgraphPreconditioner.h b/gtsam/linear/SubgraphPreconditioner.h index 9e005dd53d..558a80bd86 100644 --- a/gtsam/linear/SubgraphPreconditioner.h +++ b/gtsam/linear/SubgraphPreconditioner.h @@ -125,6 +125,9 @@ namespace gtsam { /** Apply operator A in place: needs e allocated already */ void multiplyInPlace(const VectorValues& y, Errors& e) const; + /** Apply operator A' */ + VectorValues operator^(const Errors& e) const; + /** * Add A'*e to y * y += alpha*A'*[e1;e2] = [alpha*e1; alpha*inv(R1')*A2'*e2] diff --git a/gtsam/linear/iterative.h b/gtsam/linear/iterative.h index ff3d738019..9bd1831e97 100644 --- a/gtsam/linear/iterative.h +++ b/gtsam/linear/iterative.h @@ -59,6 +59,11 @@ namespace gtsam { /** Access b vector */ const Vector& b() const { return b_; } + /** Apply operator A'*e */ + Vector operator^(const Vector& e) const { + return A_.transpose() * e; + } + /** * Print with optional string */ From 79960095b9d8e03dd9ee83d7e0c9cab810a05a72 Mon Sep 17 00:00:00 2001 From: Tyler Veness Date: Fri, 10 Jan 2025 15:36:30 -0800 Subject: [PATCH 3/3] Fix invalid use of operator^ in matrix test --- gtsam/base/tests/testMatrix.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gtsam/base/tests/testMatrix.cpp b/gtsam/base/tests/testMatrix.cpp index 4c8808722c..ede59a204b 100644 --- a/gtsam/base/tests/testMatrix.cpp +++ b/gtsam/base/tests/testMatrix.cpp @@ -567,7 +567,7 @@ TEST(Matrix, matrix_vector_multiplication ) Vector AtAv = Vector3(142., 188., 234.); EQUALITY(A*v,Av); - EQUALITY(A^Av,AtAv); + EQUALITY(A.transpose() * Av,AtAv); } /* ************************************************************************* */