Skip to content

Commit 3e11ebd

Browse files
changh95aaron-skydio
authored andcommitted
[SymForce-External] Check sparse matrix condition for Jacobian and Hessian constructors
Check sparse matrix condition for Jacobian and Hessian constructors Hi, a keen user of symforce here! Found some code that may lead to some confusion in the future. Suggesting a change for that. Best, changh95 --- - Variable `jacobian_is_sparse` should really be `kIsSparseEigenType<T>()` instead of `!kIsEigenType<T>()` - So is for `hessian_is_sparse` Closes #293 GitOrigin-RevId: d7ba870631c50db75fc1f24a8886ad2c98d29163
1 parent 783ede7 commit 3e11ebd

File tree

1 file changed

+3
-3
lines changed

1 file changed

+3
-3
lines changed

symforce/opt/factor.tcc

+3-3
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ Factor<Scalar> Factor<Scalar>::Jacobian(Functor&& func, const std::vector<Key>&
5555
"JacobianMat (last argument) should be an Eigen::Matrix or Eigen::SparseMatrix");
5656

5757
// Check sparse vs dense
58-
constexpr bool is_sparse = !kIsEigenType<JacobianMat>;
58+
constexpr bool is_sparse = kIsSparseEigenType<JacobianMat>;
5959

6060
// Get dimensions
6161
constexpr int M = ResidualVec::RowsAtCompileTime;
@@ -113,8 +113,8 @@ Factor<Scalar> Factor<Scalar>::Hessian(Functor&& func, const std::vector<Key>& k
113113
static_assert(kIsEigenType<RhsVec>, "RhsVec (last argument) should be an Eigen::Matrix");
114114

115115
// Check sparse vs dense
116-
constexpr bool jacobian_is_sparse = !kIsEigenType<JacobianMat>;
117-
constexpr bool hessian_is_sparse = !kIsEigenType<HessianMat>;
116+
constexpr bool jacobian_is_sparse = kIsSparseEigenType<JacobianMat>;
117+
constexpr bool hessian_is_sparse = kIsSparseEigenType<HessianMat>;
118118
static_assert(jacobian_is_sparse == hessian_is_sparse,
119119
"Matrices cannot be mixed dense and sparse.");
120120

0 commit comments

Comments
 (0)