-
Notifications
You must be signed in to change notification settings - Fork 52
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rescale colmap keypoints. #817
base: master
Are you sure you want to change the base?
Conversation
Do the results when loading in the COLMAP database now align with what you get when running GTSfM with the COLMAP frontend (i.e., running with the |
@travisdriver Hi Travis, apologies for the delayed response; I haven’t checked this account in a while. Are you asking whether the results from running GTSfM with the COLMAP frontend align with the results from running the full end-to-end COLMAP pipeline? |
) | ||
return Keypoints(coordinates=coordinates, scales=None, responses=None) | ||
|
||
def _read_image_ids_and_keypoints( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are you using the same formatting settings? please refer to https://github.com/borglab/gtsfm/blob/master/CONTRIBUTING.md#python-style
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there are still some format changes that are undesirable. I have highlighted 3 of them. You can be sure the right formatting settings are being used if you reformat and those changes get reverted.
two_view_geometry = self._pycolmap_db.read_two_view_geometry(colmap_i1, colmap_i2) | ||
two_view_geometry = self._pycolmap_db.read_two_view_geometry( | ||
colmap_i1, colmap_i2 | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
undesirable format change.
|
||
# Only read matches if we have an essential or a fundamental matrix | ||
if two_view_geometry.config != 2 and two_view_geometry.config != 3: | ||
continue | ||
|
||
# Note(Ayush): the matches we are loading are actually post verification | ||
corr_idxs[(i1, i2)] = np.array(two_view_geometry.inlier_matches, dtype=np.int32) | ||
corr_idxs[(i1, i2)] = np.array( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
undesirable format change.
@@ -107,7 +160,9 @@ def generate_correspondences( | |||
# Note: we will end up reading verified correspondences from the colmap DB. | |||
images_actual = client.gather(images) | |||
|
|||
gtsfm_id_to_pycolmap_id, keypoints = self._read_image_ids_and_keypoints(images_actual) | |||
gtsfm_id_to_pycolmap_id, keypoints = self._read_image_ids_and_keypoints( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
undesirable format change.
The purpose of this PR is to enable running GTSFM with feature extraction results from COLMAP's front end. COLMAP front-end results are typically stored in a database format. The
colmap_correspondence_generator.py
library aim to load COLMAP's feature extraction results directly from this database.However, there is a discrepancy in how COLMAP handles feature extraction and storage:
Key Issues Addressed by this PR:
Important Notes: