AstroNet mesh frontend metrics #295

travisdriver · 2021-09-14T04:53:51Z

This PR adds a front-end evaluation based on a provided ground truth surface mesh of the scene. High-fidelity ground truth shape models are available for small bodies explored by past missions, allowing for highly accurate evaluation of the correspondences computes by the front-end.

Through testing, the current Sampson distance (SD) method seems to consistently over-estimate the precision of the front end, at least for the AstroNet examples I have run. For example, on a 14-image sequence from Dawn @ 4 Vesta, the Sampson distance method estimated an overall precision of .9998 while the mesh-based method computed a precision of .9388.

Moreover, I ran GTSfM using the ground truth information to verify correspondences using both the SD and mesh-based methods as a sanity check. When correspondences were verfied using the ground truth mesh, the final reconstruction was perfect. However, the result using the SD method was far from perfect.

I still think the Sampson distance is a useful metric for when a ground truth mesh is not present, but I think the mesh-based front-end evaluation provides a more accurate estimate of the precision

Mesh-based method

mean_inlier_ratio_wrt_gt_model: 0.9998

Metrics for pair (0, 8)

(0, 8)
"rotation_angular_error": 15.69,
"translation_angular_error": 8.21,
"num_inliers_gt_model": 376,
 "inlier_ratio_gt_model": 0.82,
"inlier_avg_reproj_error_gt_model": 1.86,
 "outlier_avg_reproj_error_gt_model": 7.34,
"inlier_ratio_est_model": 0.78,
 "num_inliers_est_model": 459

Final reconstruction with ground truth verified correspondences

Sampson distance method

mean_inlier_ratio_wrt_gt_model: 0.9388

Metrics for pair (0, 8)

(0, 8)
"rotation_angular_error": 15.69,
"translation_angular_error": 8.21,
"num_inliers_gt_model": 459,
"inlier_ratio_gt_model": 1.0,
"inlier_avg_reproj_error_gt_model": 0.63,
"outlier_avg_reproj_error_gt_model": NaN,
"inlier_ratio_est_model": 0.78,
"num_inliers_est_model": 459

Final reconstruction with ground truth verified correspondences

…nsistency

gtsfm/utils/metrics.py

visualization/open3d_vis_utils.py

visualization/view_scene.py

gtsfm/utils/metrics.py

tests/utils/test_metric_utils.py

gtsfm/common/two_view_estimation_report.py

tests/utils/test_metric_utils.py

ayushbaid

Looks good overall. My main concern is that why we compute reprojection errors only on points which are inliers from epipolar constraint.

gtsfm/loader/astronet_loader.py

ayushbaid · 2021-11-04T04:43:11Z

gtsfm/runner/run_scene_optimizer_astronet.py

+
+        with Client(cluster) as client, performance_report(filename="dask-report.html"):
+            # Scatter surface mesh across all nodes to preserve computation time and memory.
+            gt_scene_trimesh_future = client.scatter(self.loader.gt_scene_trimesh, broadcast=True)


This is interesting. Do you have any experiments/rough approximations on the savings obtained by scatter?

If you recommend, we can use this scatter at other places too (like NN models)

To be honest, the main reason I did it is because Dask told me to, but I can definitely say it saves a significant amount of computation time (although I don't have any exact numbers).

The documentation is a little spotty, but from my understanding, it preallocates memory for the object on each worker and then passes around the pointer to the worker threads.

It definitely may be useful for other large objects in the pipeline.

gtsfm/scene_optimizer.py

ayushbaid · 2021-11-04T05:22:30Z

gtsfm/two_view_estimator.py

+        num_inliers_gt_model = np.count_nonzero(v_corr_idxs_inlier_mask_gt)
+        inlier_ratio_gt_model = (
+            np.count_nonzero(v_corr_idxs_inlier_mask_gt) / v_corr_idxs.shape[0] if len(v_corr_idxs) > 0 else 0.0
+        )


this should just depend on v_corr_idxs_inlier_mask_gt not being None

Yeah I'm getting a lot of mypy errors for a lot of variables because many of them are labeled as Optional. I think this should be part of a larger effort where we reassess whether certain variables are, in fact, optional.

gtsfm/two_view_estimator.py

gtsfm/utils/metrics.py

ayushbaid · 2021-11-04T05:59:25Z

gtsfm/utils/metrics.py

+    return is_inlier, reproj_err
+
+
+def compute_keypoint_intersections(


Yes, I live putting it in the new file. This is a function which can potentially be used in other components too (in the future).

travisdriver · 2021-11-04T15:23:44Z

Looks good overall. My main concern is that why we compute reprojection errors only on points which are inliers from epipolar constraint.

The reprojection errors are computed for all keypoints, not just inliers.

gtsfm/common/two_view_estimation_report.py

johnwlambert · 2021-11-04T19:37:07Z

gtsfm/scene_optimizer.py

+                if report.reproj_error_gt_model is not None and report.v_corr_idxs_inlier_mask_gt is not None
+                else None,
+                "outlier_avg_reproj_error_gt_model": round(
+                    np.nanmean(report.reproj_error_gt_model[np.logical_not(report.v_corr_idxs_inlier_mask_gt)]),


@travisdriver can't we just use report.inlier_avg_reproj_error_gt_model here? why do we need to compute the mean over again? I think we already compute it in the TwoViewEstimator

https://github.com/borglab/gtsfm/pull/295/files?authenticity_token=sUeFac35fdQHIM%2FWNeNuKnBbZw%2Bq8phgw%2B4iKHfmflK7LqGME7HzIYxnQRv3OJqxvyVxjM1akbCEnMutH8PLxg%3D%3D&file-filters%5B%5D=.py#diff-1f5b63f35ed4d88709ce154ae9808f38a1a7178fb5f74211b1053ae5a8c37894R177

You're right. I think it's cleaner to just compute it here when everything else is computed. I'll remove it here

Actually I'm going to open a different PR to cleaner up the TwoViewEstimationReport a little

johnwlambert · 2021-11-04T19:43:03Z

gtsfm/two_view_estimator.py

-        rot3_angular_errors.append(report.R_error_deg)
-        trans_angular_errors.append(report.U_error_deg)
+        if report.R_error_deg is not None:
+            rot3_angular_errors.append(report.R_error_deg)


@travisdriver we shouldn't need this is not None check here, since we are just filling the array with None, and then casting it to float which converts them to Nan, and then they are ignored in the np.nanmean call.

But if keeps the types simpler, maybe it's ok.

Was trying to fix some of the mypy errors

Got it, sure.

johnwlambert · 2021-11-04T19:52:54Z

gtsfm/utils/metrics.py

+            gt_scene_mesh,
+            dist_threshold,
+        )
+    elif gt_wTi1 is not None and gt_wTi2 is not None:


i don't think we need this check here to see if the ground truth poses are provided since we already check it in the TwoViewEstimator:

https://github.com/borglab/gtsfm/pull/295/files?authenticity_token=QXZc5PBgvc%2BslrvBTHXtF%2BPMIrnyqmt7Pn2kCL1%2B2atLH3gBLigzOifRNQyOrhmMLLI0OpUCYqt5D02N1VtsPw%3D%3D&file-filters%5B%5D=.py#diff-1f5b63f35ed4d88709ce154ae9808f38a1a7178fb5f74211b1053ae5a8c37894R120

i think the double nesting here makes it harder to read. I prefer just checking to see if the mesh is there. or you can exit immediately with None,None.

We discuss this a but in Contributing.md, but returning early is always preferred to nesting.

johnwlambert · 2021-11-05T02:24:57Z

gtsfm/two_view_estimator.py

+    # Compute ground truth metrics.
+    if v_corr_idxs_inlier_mask_gt is not None and reproj_error_gt_model is not None:
+        num_inliers_gt_model = np.count_nonzero(v_corr_idxs_inlier_mask_gt)
+        inlier_ratio_gt_model = (


@travisdriver this is actually a different definition of the metric than the one we have been using. Before we were using the # of putatives as the denominator

https://github.com/borglab/gtsfm/pull/295/files#diff-1f5b63f35ed4d88709ce154ae9808f38a1a7178fb5f74211b1053ae5a8c37894L223

The way we had it before follows Heinly12eccv:
https://www.cs.unc.edu/~jheinly/publications/eccv2012-heinly.pdf

If we try to compute inlier_ratio_gt_model here, we don't have access to the # of putatives (corr_idxs instead of v_corr_idxs)

Maybe that metric should be moved to the matcher, actually, since it's a matcher-based metric, and has nothing to do with the verifier.

It looks like the fraction here is (#verified w.r.t. GT model) / (# verified w.r.t. estimated model), which I don't think is what we want for inlier ratio, since it could be greater than 1.

Actually, it looks like we were computing it this way before, since we were passing in v_corr_idxs to compute_correspondence_metrics(), not corr_idxs.

So it looks like the inlier_ratio_gt_model here is actually

inlier_ratio_gt_model = (# verified correspondences that were right w.r.t. GT model) / # verrified correspondences

@akshay-krishnan I think this explains the bug we saw in #306, actually

travisdriver added 2 commits September 14, 2021 00:48

working on 2011212_opnav_022

497e0cf

removed dask report

9315b2a

travisdriver requested review from akshay-krishnan, ayushbaid and johnwlambert September 14, 2021 04:53

travisdriver added 4 commits September 15, 2021 00:28

added inlier mask to correspondence visualization

52d1975

removed erroneous files

d13676c

hyperparameter testing

cc2f8f7

merged master

185a161

travisdriver linked an issue Sep 18, 2021 that may be closed by this pull request

Use ground truth shape models and AstroNet data to evaluate frontend #256

Closed

2 tasks

travisdriver added 9 commits September 19, 2021 16:01

using gtsam functions for forward and backward projection

20b32c8

fixed merge conflicts, haven't tested yet

b1343ed

merged with master

bfcd6d0

runs but doesn't compute metrics

93c1aa4

computing metrics, need to add recall

73a6a94

added trimesh to environment_linux

f6597f0

remove changes to deep config, remove use of GT poses before cycle co…

f252378

…nsistency

remove changes to config

2eec798

pushing again to run workflow

7fcc9f8

travisdriver mentioned this pull request Oct 11, 2021

Color inliers and outliers (w.r.t. GT poses) green/red, and make CI upload these images as artifacts. #343

Merged

working with GTSAM functions, added ground truth verifier

a0637c6

travisdriver mentioned this pull request Oct 13, 2021

Use robust shonan #344

Closed

johnwlambert reviewed Oct 13, 2021

View reviewed changes

gtsfm/utils/metrics.py Show resolved Hide resolved

johnwlambert reviewed Oct 13, 2021

View reviewed changes

gtsfm/utils/metrics.py Show resolved Hide resolved

johnwlambert reviewed Oct 13, 2021

View reviewed changes

gtsfm/utils/metrics.py Outdated Show resolved Hide resolved

johnwlambert reviewed Oct 13, 2021

View reviewed changes

gtsfm/utils/metrics.py Show resolved Hide resolved

johnwlambert reviewed Oct 13, 2021

View reviewed changes

gtsfm/utils/metrics.py Outdated Show resolved Hide resolved

johnwlambert reviewed Oct 13, 2021

View reviewed changes

gtsfm/utils/metrics.py Outdated Show resolved Hide resolved

johnwlambert reviewed Oct 13, 2021

View reviewed changes

visualization/open3d_vis_utils.py Outdated Show resolved Hide resolved

johnwlambert reviewed Oct 13, 2021

View reviewed changes

visualization/view_scene.py Outdated Show resolved Hide resolved