-
Notifications
You must be signed in to change notification settings - Fork 1.4k
WIP : support for hloc toolbox (superpoint, superglue, netvlad,...) #1026
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
WIP : support for hloc toolbox (superpoint, superglue, netvlad,...) #1026
Conversation
…olmap), just a first test, not ready yet
pull from head repository
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.
Thanks for adding support for hloc. My main comment atm is around notifying users how to install hloc if it is not included. We should also add documentation for these alternative matches in the web docs.
ok, I'll prepare a new version with these try/except for dependency, notification, and documentation update |
merge from main repo
I updated the PR : The documentation of ns-process-data has a new line "You may also want to install hloc for more feature detector and matcher options." with a link to the github page. The new cli arguments (sfm_tool, feature_type, matcher_type) have a description in the doc too. |
merge from main repo
Fix torch spherical harmonics (nerfstudio-project#1054)
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.
Thanks for the updates! Almost ready, just a few small additional comments.
@@ -127,7 +103,7 @@ def main(self) -> None: | |||
if not self.skip_colmap: | |||
colmap_dir.mkdir(parents=True, exist_ok=True) | |||
|
|||
(sfm_tool, feature_type, matcher_type) = find_tool_feature_matcher_combination( | |||
(sfm_tool, feature_type, matcher_type) = process_data_utils.find_tool_feature_matcher_combination( |
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.
Can you raise an error if no valid tool is found.
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.
find_tool_feature_matcher_combination returns (None,None,None) when no valid tool is found, which brings to the message CONSOLE.log("[bold yellow]Warning: Invalid combination of sfm_tool, feature_type, and matcher_type ").
I'll add a sys.exit(1) after the message is printed.
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.
I changed the message to "[bold red]Invalid combination of sfm_tool, feature_type, and matcher_type, exiting" and added sys.exit(1)
@@ -251,7 +227,7 @@ def main(self) -> None: | |||
if not self.skip_colmap: | |||
colmap_dir.mkdir(parents=True, exist_ok=True) | |||
|
|||
(sfm_tool, feature_type, matcher_type) = find_tool_feature_matcher_combination( | |||
(sfm_tool, feature_type, matcher_type) = process_data_utils.find_tool_feature_matcher_combination( |
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.
Same here
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.
same as other message
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.
LGTM, Thanks for adding this functionality! Sorry for the delayed reviews.
…erfstudio-project#1026) * basic support for hloc (includes netvlad, super point, superglue in colmap), just a first test, not ready yet * add exhaustive match option for hloc * add command line option to choose the type of feature, matcher,... * make hloc an optional dependency, update doc, and small bug fixes * fix missing imports * fix warnings, pylint,... * fix overwrite previous commits * fix import order from isort * move find_tool_feature_matcher_combination to process_data_utils, add type hints and Args/Returns doc * error message when no valid combination of sfm tool, feature type, and matcher Co-authored-by: RandomPrototypes <[email protected]>
When I use the command: This error occurred: These are complete information: Does anybody meet the same problem? Know the reason or know how to solve this? ? It is important and urgent for me!!! Thanks a lot!!! |
…erfstudio-project#1026) * basic support for hloc (includes netvlad, super point, superglue in colmap), just a first test, not ready yet * add exhaustive match option for hloc * add command line option to choose the type of feature, matcher,... * make hloc an optional dependency, update doc, and small bug fixes * fix missing imports * fix warnings, pylint,... * fix overwrite previous commits * fix import order from isort * move find_tool_feature_matcher_combination to process_data_utils, add type hints and Args/Returns doc * error message when no valid combination of sfm tool, feature type, and matcher Co-authored-by: RandomPrototypes <[email protected]>
This adds support for hloc which brings support for more recent features and matches such as superpoint and superglue.
I added command-line options to select the sfm toolbox (colmap, hloc), feature (sift, superpoint,...), and matcher (NN, superglue,...).
It requires hloc as dependency.