From 24ccdce95dc84c52420060de920f3836d2c008cf Mon Sep 17 00:00:00 2001
From: akshay-krishnan <akshay.krishnan.30@gmail.com>
Date: Sat, 23 Nov 2024 15:18:32 -0500
Subject: [PATCH] fix to actually use relative rotation errors

---
 gtsfm/evaluation/compare_colmap_outputs.py | 15 ++++++++++-----
 gtsfm/utils/metrics.py                     | 10 ++++------
 2 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/gtsfm/evaluation/compare_colmap_outputs.py b/gtsfm/evaluation/compare_colmap_outputs.py
index aa2d22643..038985254 100644
--- a/gtsfm/evaluation/compare_colmap_outputs.py
+++ b/gtsfm/evaluation/compare_colmap_outputs.py
@@ -25,7 +25,6 @@
 def load_poses(colmap_dirpath: str) -> Dict[str, Pose3]:
     """Returns mapping from image filename to associated camera pose."""
     wTi_list, img_fnames, _, _, _, _ = io_utils.read_scene_data_from_colmap_format(colmap_dirpath)
-    # img_fnames = [f.split("/")[-1] for f in img_fnames]
 
     return dict(zip(img_fnames, wTi_list))
 
@@ -85,12 +84,18 @@ def compare_poses(baseline_dirpath: str, eval_dirpath: str, output_dirpath: str)
     metrics = []
     metrics.append(metric_utils.compute_rotation_angle_metric(wRi_aligned_list, baseline_wRi_list))
     metrics.append(metric_utils.compute_translation_distance_metric(wti_aligned_list, baseline_wti_list))
-    metrics.append(metric_utils.compute_relative_translation_angle_metric(i2Ui1_dict_gt, current_wTi_list))
     metrics.append(metric_utils.compute_translation_angle_metric(baseline_wTi_list, current_wTi_list))
-    metrics.append(metric_utils.compute_relative_translation_angle_metric(i2Ri1_dict_gt, current_wTi_list))
+    relative_rotation_error_metric = metric_utils.compute_relative_rotation_angle_metric(
+        i2Ri1_dict_gt, current_wTi_list
+    )
+    metrics.append(relative_rotation_error_metric)
+    relative_translation_error_metric = metric_utils.compute_relative_translation_angle_metric(
+        i2Ui1_dict_gt, current_wTi_list
+    )
+    metrics.append(relative_translation_error_metric)
 
-    rotation_angular_errors = metrics[2]._data
-    translation_angular_errors = metrics[4]._data
+    rotation_angular_errors = relative_rotation_error_metric._data
+    translation_angular_errors = relative_translation_error_metric._data
     metrics.extend(
         metric_utils.compute_pose_auc_metric(
             rotation_angular_errors, translation_angular_errors, save_dir=output_dirpath
diff --git a/gtsfm/utils/metrics.py b/gtsfm/utils/metrics.py
index dd20396a6..787efa989 100644
--- a/gtsfm/utils/metrics.py
+++ b/gtsfm/utils/metrics.py
@@ -281,8 +281,6 @@ def compute_relative_rotation_angle_metric(
     angles: List[Optional[float]] = []
     for i1, i2 in i2Ri1_dict:
         i2Ri1 = i2Ri1_dict[(i1, i2)]
-        # TODO(akshay-krishnan): this is wrong, we should not penalize for i2Ri1 (GT) being None.
-        # added now to be consistent with translation.
         if i2Ri1 is None or wTi_list[i1] is None or wTi_list[i2] is None:
             angles.append(None)
             continue
@@ -290,7 +288,7 @@ def compute_relative_rotation_angle_metric(
         wRi2_gt = wTi_list[i2].rotation()
         i2Ri1_gt = wRi2_gt.inverse().compose(wRi1_gt)
         angles.append(comp_utils.compute_relative_rotation_angle(i2Ri1, i2Ri1_gt))
-    return GtsfmMetric("relative_translation_angle_error_deg", np.array(angles, dtype=np.float32))
+    return GtsfmMetric("relative_rotation_angle_error_deg", np.array(angles, dtype=np.float32))
 
 
 def compute_translation_angle_metric(
@@ -325,8 +323,8 @@ def compute_translation_angle_metric(
 
 
 def compute_pose_auc_metric(
-    rotation_angular_errors: Sequence[float],
-    translation_angular_errors: Sequence[float],
+    rotation_angular_errors: Union[Sequence[float], np.ndarray],
+    translation_angular_errors: Union[Sequence[float], np.ndarray],
     thresholds_deg: Tuple[float] = (1, 2.5, 5, 10, 20),
     save_dir: Optional[str] = None,
 ) -> List[GtsfmMetric]:
@@ -394,7 +392,7 @@ def compute_ba_pose_metrics(
 def get_all_relative_rotations_translations(
     wTi_list: List[Optional[Pose3]],
 ) -> Tuple[Dict[Tuple[int, int], Optional[Rot3]], Dict[Tuple[int, int], Optional[Unit3]]]:
-    """Generate synthetic measurements of the 2-view translation directions between image pairs.
+    """Compute measurements of *all* 2-view translation directions between image pairs.
 
     Args:
         wTi_list: List of poses (e.g. could be ground truth).