MichaelGrupp / evo

Python package for the evaluation of odometry and SLAM
https://michaelgrupp.github.io/evo/
GNU General Public License v3.0
3.34k stars 745 forks source link

Documentation might be wrong #553

Closed stefangachter closed 8 months ago

stefangachter commented 1 year ago

Hi Michael, I am a little bit puzzled but came across the following: The documentation here https://github.com/MichaelGrupp/evo/blob/master/notebooks/metrics.py_API_Documentation.ipynb states: image That is, the difference between the estimated and reference pose is the inverse reference pose times the estimated pose. This definition results in an error in the local frame. However, if I dig into the code, then the error in the global frame is computed:

def ape(traj_ref: PosePath3D, traj_est: PosePath3D ...
...
data = (traj_ref, traj_est)
...
ape_metric.process_data(data)
def process_data(self, data: PathPair) -> None:
...
traj_ref, traj_est = data
....
self.E = [self.ape_base(x_t, x_t_star) for x_t, x_t_star in zip(traj_est.poses_se3, traj_ref.poses_se3)]

If I am not mistaken, then x_t := x_est_t and x_t_star := x_ref_t

def ape_base(x_t: np.ndarray, x_t_star: np.ndarray) -> np.ndarray:
....
return lie.relative_se3(x_t, x_t_star)
def relative_se3(p1: np.ndarray, p2: np.ndarray) -> np.ndarray:
    """
    :param p1, p2: SE(3) matrices
    :return: the relative transformation p1^{⁻1} * p2
    """
    return np.dot(se3_inverse(p1), p2)

Again, if I am not mistaken, then p1 := x_est_t and p2 := x_ref_t

Thus x_est_t^{-1} * x_ref_t but the documentation states the opposite. Hopefully, I didn't overlook something.

MichaelGrupp commented 1 year ago

It doesn't matter because the final value of the metrics always involves a norm (see the part below the formula you referenced).

That's also the reason why you can swap the two input trajectories and still get the same result.

stefangachter commented 1 year ago

I agree with you and I am aware of this fact. However, if you would like to go a step beyond and compare the error with the estimated uncertainties, then it is highly relevant how the error is computed. This is not (yet) part of evo but would be a desirable feature. In my view, any researcher doing estimation should go beyond and have a look at the uncertainties as well. Thus, the documentation and code should be consistent such that anybody using the outcome of evo and would like to go "beyond" is guided "correctly".

MichaelGrupp commented 8 months ago

Changed the order in the notebook now: https://github.com/MichaelGrupp/evo/commit/47c96ae68fab14bdaea059776fff31a86ccf0305

Thanks!