WIP: Resolve "Remove wolf::Scalar and use double instead"

Closes #264 (closed)

added Code organization External API update To Do (release) labels

added 2 commits

• 09970eb5 - Explicit Scalar --> double
• ac32c2a7 - Remove typedefs of Scalar type

Compare with previous version

added 1 commit

• 04fe2bd8 - Update wolf.h

Compare with previous version

@joanvallve we are having ctest fails in this branch related I think to factorAutodiff. I have not been able to find the problem. Would you mind having a look?

The failing tests are:

The following tests FAILED:
	  3 - gtest_factor_autodiff (Failed)
	 27 - gtest_factor_absolute (Failed)
	 30 - gtest_factor_odom_3D (Failed)
	 32 - gtest_factor_pose_3D (Failed)

All of them are deriving from factorAutodiff, so that's why I think the problem is there.

This MR in principle only substituted Scalar --> double and removed all Eigen typedefs related to Scalar... it's a little strange that we have these ctest fails...

Thanks!

This is because this change removed the RowMajor definition of the matrices. Then jacobians are stored in colmajor while Ceres provides them as arrays row by row.

Wow! True, I would have never found it!

Does this have any drawbacks?

So... what to do?

In the meantime, I removed all the remaining RowMajor to have all typedefs equal. We had in Wolf defined Matrix1s and Matrix6s which are not in the standard Eigen.

Also, SparseMatrix is now ColMajor.

But I dont know how to proceed from here.

added 1 commit

• 18e5cdbd - Remove RowMajor

Compare with previous version

I guess it's this code here in FactorAutodiff::evaluate():

// fill the jacobian matrices
for (unsigned int i = 0; i<n_blocks; i++)
    if (jacobians[i] != nullptr)
        for (unsigned int row = 0; row < RES; row++)
            std::copy((*residuals_jets_)[row].v.data() + jacobian_locations_.at(i),
                      (*residuals_jets_)[row].v.data() + jacobian_locations_.at(i) + state_block_sizes_.at(i),
                      jacobians[i] + row * state_block_sizes_.at(i));

yes, see #217 (closed), this is an open issue..

In other places, e.g. here they define row-major matrices just inside Evaluate().

added 6 commits

• 18e5cdbd...13ad32b9 - 5 commits from branch devel
• 704697f5 - Merge branch 'devel' into 264-remove-wolf-scalar-and-use-double-instead

Compare with previous version

This is related with the current/future implementation of our factors.

The code you link is an example of a factor that is not "extracting" the Jacobians to the outside, they are only used inside the evaluate function.

In our case, we have two different evaluate functions in FactorBase:

• virtual bool evaluate(double const* const* _parameters, double* _residuals, double** _jacobians) const = 0;
• virtual void evaluate(const std::vector<const double*>& _states_ptr, Eigen::VectorXd& _residual, std::vector<Eigen::MatrixXd>& _jacobians) const = 0;

The first one is ceres-friendly, the Jacobians are computed using the jets (wolf re-implementation). Here, we do not use Eigen.

If I remember it properly, the second one is to get the Jacobians and the residuals evaluated at some point. This is used by the FactorAnalytic and other things (pruning..).

This second function would be the one that we have to redesign with the new factor API.

We are a little stuck with this MR, I dont know what to do regarding the discussion above about row/col major

added 4 commits

• 731737d3 - fixing rowmajor bug
• 8f66de29 - eigen map to avoid useless copies
• cd71e44f - local param using rowmajor map for Jacobians
• c9cf6321 - factor auto diff rowmajor changes

Compare with previous version

added 9 commits

• c9cf6321...26c15b59 - 8 commits from branch devel
• b17a2329 - Merge branch 'devel' into 264-remove-wolf-scalar-and-use-double-instead

Compare with previous version

merged