8000 Split pair generation and matching by sarlinpe · Pull Request #2573 · colmap/colmap · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Split pair generation and matching #2573

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

Merged
merged 20 commits into from
May 24, 2024
Merged

Split pair generation and matching #2573

merged 20 commits into from
May 24, 2024

Conversation

sarlinpe
Copy link
Member

This is an attempt at splitting the generation of image pairs (exhaustive, spatial, vocabulary tree, sequential) and the matching process, which were deeply entangled. This simplifies the code and makes it possible to generate the pairs from Python for a subsequent Python-based matching.

The design is sometimes convoluted because I make sure to avoid materializing all pairs in memory, and instead generate them by batches.

This currently lives in controllers/ but could be moved to features/.

@sarlinpe sarlinpe requested a review from ahojnnes May 22, 2024 12:08
}

class ExhaustiveFeatureMatcher : public Thread {
template <typename DerivedPairGenerator>
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess instead of templating we could pass an abstract pair generator and the cache object from the outside. Not necessarily important for here but theoretically has some benefits for injecting a mock/fake for testing purposes of this class.

Copy link
Contributor
@ahojnnes ahojnnes left a comment

Choose a reason for hiding this comment

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

Thanks, looks like a good refactor to me. I am also fine with the proposed move from controllers/ to features/. I guess we could now even write some simple tests for the pair generation - not a blocker for this PR though, as there weren't before either.

@ahojnnes
Copy link
Contributor
ahojnnes commented May 24, 2024

@sarlinpe Mac is unhappy with the following error:

undefined colmap::(anonymous namespace)::FeaturePairsFeatureMatcher::kCacheSize

@sarlinpe sarlinpe enabled auto-merge (squash) May 24, 2024 10:12
@sarlinpe sarlinpe merged commit 022e42f into main May 24, 2024
15 checks passed
@sarlinpe sarlinpe deleted the sarlinpe/refactor-matching branch May 24, 2024 10:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0