Add support to run multiple correspondence generator before 2-view estimation #663

ayushbaid · 2023-07-22T23:30:20Z

No description provided.

ayushbaid · 2023-07-24T01:40:33Z

gtsfm/runner/gtsfm_runner_base.py

+        # TODO(Handle this)
+        # process_graph_generator = ProcessGraphGenerator()
+        # if isinstance(self.scene_optimizer.correspondence_generator, ImageCorrespondenceGenerator):
+        #     process_graph_generator.is_image_correspondence = True
+        # process_graph_generator.save_graph()


@stepanyanhayk How should I handle having multiple correspondence generators?

Why is this commented out?

When we have multiple correspondence generator, I cannot use the single flag to control the process graph generation. I need to upgrade the process graph generator too.

ayushbaid · 2024-01-02T16:36:46Z

Metrics on 01/02: visual_comparison_dashboard.zip

travisdriver · 2024-01-25T22:28:30Z

Some unit tests failing to do FAILED tests/test_gtsfm_runner_olssonloader.py::test_gtsfm_runner_olssonloader[dataset_path0] - hydra.errors.MissingConfigException: Cannot find primary config 'sift_front_end.yaml'. Check that it's in your config search path.

travisdriver · 2024-01-25T22:29:16Z

.github/workflows/benchmark.yml

+            # config              dataset                lookahead   img-extension  source      loader          max-res  share-intrinsics append-sift
+            [sift,                door-12,               15,         JPG,           test_data,  olsson-loader,  1296,    true,    false ],
+            [lightglue,           door-12,               15,         JPG,           test_data,  olsson-loader,  1296,    true,    true  ],
+            [sift,                skydio-8,              15,         jpg,           gdrive ,    colmap-loader,  760,     true,    false ],
+            [lightglue,           skydio-8,              15,         jpg,           gdrive,     colmap-loader,  760,     true,    true  ],
+            [sift,                skydio-32,             15,         jpg,           gdrive,     colmap-loader,  760,     true,    false ],
+            [lightglue,           skydio-32,             15,         jpg,           gdrive,     colmap-loader,  760,     true,    true  ],
+            [sift,                palace-fine-arts-281,  15,         jpg,           wget,       olsson-loader,  320,     true,    false ],
+            [lightglue,           notre-dame-20,         15,         jpg,           gdrive,     colmap-loader,  760,     false,   true  ],
+            [sift,                2011205_rc3,           15,         png,           wget,       astrovision,    1024,    true,    false ],
+            [lightglue,           2011205_rc3,           15,         png,           wget,       astrovision,    1024,    true,    true ],
+            [sift,                gerrard-hall-100,      15,         jpg,           wget,       colmap-loader,  760,     true,    false ],
+            [lightglue,           gerrard-hall-100,      15,         jpg,           wget,       colmap-loader,  760,     true,    true ],
+            [sift,                south-building-128,    15,         jpg,           wget,       colmap-loader,  760,     true,    false ],
+            [lightglue,           south-building-128,    15,         jpg,           wget,       colmap-loader,  760,     true,    true ],


Make these all false for now

travisdriver · 2024-01-25T22:30:19Z

gtsfm/configs/onedsfm_front_end.yaml

+  correspondence_generators:
+    - _target_: gtsfm.frontend.correspondence_generator.det_desc_correspondence_generator.DetDescCorrespondenceGenerator

-    detector_descriptor:
-      _target_: gtsfm.frontend.cacher.detector_descriptor_cacher.DetectorDescriptorCacher
-      detector_descriptor_obj:
-        _target_: gtsfm.frontend.detector_descriptor.sift.SIFTDetectorDescriptor
-        max_keypoints: 5000
+      detector_descriptor:
+        _target_: gtsfm.frontend.cacher.detector_descriptor_cacher.DetectorDescriptorCacher
+        detector_descriptor_obj:
+          _target_: gtsfm.frontend.detector_descriptor.sift.SIFTDetectorDescriptor
+          max_keypoints: 5000

-    matcher:
-      _target_: gtsfm.frontend.cacher.matcher_cacher.MatcherCacher
-      matcher_obj:
-        _target_: gtsfm.frontend.matcher.twoway_matcher.TwoWayMatcher
-        ratio_test_threshold: 0.8
+      matcher:
+        _target_: gtsfm.frontend.cacher.matcher_cacher.MatcherCacher
+        matcher_obj:
+          _target_: gtsfm.frontend.matcher.twoway_matcher.TwoWayMatcher
+          ratio_test_threshold: 0.8


Why the extra indent?

Because now correspondence_generators is a list. You'll notice an extra - at line 20. Each entry is an item in the list too.

travisdriver · 2024-01-25T22:33:42Z

gtsfm/runner/gtsfm_runner_base.py

+            ) = correspondence_utils.merge_correspondences(
+                keypoints_from_all_corr_generators,
+                putatutive_corr_idxs_from_all_corr_generators,


What if there's only one generator?

Could we avoid calling this unless generators > 1?

Good point.

Actually, the function will be a no-op and just return early if we have just 1 correspondence generator. Leaving it as is.

travisdriver · 2024-01-25T22:35:35Z

It looks like you deleted a couple of the front-end configs. Did you mean to do that?

ayushbaid · 2024-01-28T05:43:02Z

It looks like you deleted a couple of the front-end configs. Did you mean to do that?

I thought everything used unified, but apparently not. Let me revert.

johnwlambert · 2024-01-28T06:47:12Z

CI is failing btw

johnwlambert · 2024-03-12T05:36:09Z

.github/workflows/benchmark.yml

+            # config              dataset                lookahead   img-extension  source      loader          max-res  share-intrinsics append-sift
+            [sift,                door-12,               15,         JPG,           test_data,  olsson-loader,  1296,    true,            false],
+            [lightglue,           door-12,               15,         JPG,           test_data,  olsson-loader,  1296,    true,            false],
+            [sift,                skydio-8,              15,         jpg,           wget ,    colmap-loader,  760,     true,                     false],


Btw I think the indentation is uneven on this line

johnwlambert · 2024-03-12T05:37:08Z

gtsfm/utils/correspondence.py

+    )
+
+
+def merge_correspondences(


could we add a unit test for this?

johnwlambert · 2024-03-12T05:37:13Z

gtsfm/utils/correspondence.py

+from gtsfm.common.keypoints import Keypoints
+
+
+def merge_keypoints(keypoints_1: Keypoints, keypoints_2: Keypoints) -> Tuple[Keypoints, int]:


could we add a unit test for this?

ayushbaid requested a review from johnwlambert July 24, 2023 01:39

ayushbaid commented Jul 24, 2023

View reviewed changes

ayushbaid force-pushed the feature/multiple-corr-generator branch from 017f9c4 to 1a76943 Compare January 2, 2024 01:48

ayushbaid requested review from akshay-krishnan and travisdriver January 11, 2024 05:32

travisdriver reviewed Jan 25, 2024

View reviewed changes

ayushbaid added 10 commits January 28, 2024 20:37

Running multiple correspondence generators before two view estimator

1cd5880

Use LoFTR + SupeGlue in skydio_front_end

ca06c9f

Fixing post rebase

7001d24

Fix existing configs

8542fc2

Support multiple correspondence generator overrides

22a6856

Append SIFT in benchmarks

1afbeab

Delete deprecated full-configs

bad23b0

Limit number of matches from loftr

1f7f05e

Restore all configs

34c9198

Use separate config for LoFTR with keypoints limit

aac4d6b

ayushbaid force-pushed the feature/multiple-corr-generator branch from 787a42a to aac4d6b Compare January 29, 2024 04:39

ayushbaid requested a review from travisdriver January 29, 2024 04:40

Rename to correspondence_generators in frontend

3be926d

ayushbaid mentioned this pull request Mar 12, 2024

Add correspondence augmenter #658

Closed

johnwlambert reviewed Mar 12, 2024

View reviewed changes

gtsfm/utils/correspondence.py

)

def merge_correspondences(

Copy link

Collaborator

johnwlambert Mar 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we add a unit test for this?

johnwlambert reviewed Mar 12, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support to run multiple correspondence generator before 2-view estimation #663

Add support to run multiple correspondence generator before 2-view estimation #663

ayushbaid commented Jul 22, 2023

ayushbaid Jul 24, 2023

travisdriver Jan 25, 2024

ayushbaid Jan 28, 2024

ayushbaid commented Jan 2, 2024

travisdriver commented Jan 25, 2024

travisdriver Jan 25, 2024

travisdriver Jan 25, 2024

ayushbaid Jan 28, 2024

travisdriver Jan 25, 2024

travisdriver Jan 25, 2024

ayushbaid Jan 28, 2024

ayushbaid Jan 28, 2024

travisdriver commented Jan 25, 2024

ayushbaid commented Jan 28, 2024

johnwlambert commented Jan 28, 2024

johnwlambert Mar 12, 2024

johnwlambert Mar 12, 2024

johnwlambert Mar 12, 2024

		from gtsfm.common.keypoints import Keypoints


		def merge_keypoints(keypoints_1: Keypoints, keypoints_2: Keypoints) -> Tuple[Keypoints, int]:

Add support to run multiple correspondence generator before 2-view estimation #663

Are you sure you want to change the base?

Add support to run multiple correspondence generator before 2-view estimation #663

Conversation

ayushbaid commented Jul 22, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ayushbaid commented Jan 2, 2024

travisdriver commented Jan 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

travisdriver commented Jan 25, 2024

ayushbaid commented Jan 28, 2024

johnwlambert commented Jan 28, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment