Added RoMa #812
Open
Added RoMa #812
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Pull Request Overview
This PR integrates the RoMa dense feature matcher into the codebase and updates the correspondence generation pipeline to allow configurable keypoint aggregation.
- Adds RoMa as a git submodule and updates
.gitmodules. - Implements
RoMamatcher ingtsfm/frontend/matcher/roma.py. - Refactors
ImageCorrespondenceGeneratorto accept aaggregatorparameter and updates config files for RoMa and LoFTR.
Reviewed Changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| thirdparty/RoMa | Added RoMa submodule reference |
| gtsfm/frontend/matcher/roma.py | New RoMa image matcher implementation |
| gtsfm/frontend/correspondence_generator/image_correspondence_generator.py | Updated constructor to take a KeypointAggregator |
| gtsfm/configs/correspondence/roma.yaml | Configuration for RoMa-based correspondence generator |
| gtsfm/configs/correspondence/loftr.yaml | Added aggregator config key for LoFTR generator |
| .gitmodules | Registered thirdparty/RoMa submodule |
Comments suppressed due to low confidence (4)
gtsfm/frontend/matcher/roma.py:34
- The docstring refers to
use_outdoor_model, but the constructor parameter is nameduse_indoor_model. Update the doc or parameter name for consistency.
use_outdoor_model (optional): use the outdoor pretrained model. Defaults to True.
gtsfm/frontend/matcher/roma.py:36
- The docstring default for
min_confidence(0.95) does not match the code default (0.1). Please correct the documented default value.
min_confidence(optional): Minimum confidence required for matches. Defaults to 0.95.
- The key
matcher_objis inconsistent withImageCorrespondenceGenerator's constructor argumentmatcher. Rename this tomatcherto match the expected parameter name.
matcher_obj:
gtsfm/frontend/matcher/roma.py:1
- Consider adding unit tests for the new
RoMamatcher to validate integration, output shapes, and parameter configurations.
"""RoMa image matcher.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.