-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Conversation
} | ||
|
||
class ExhaustiveFeatureMatcher : public Thread { | ||
template <typename DerivedPairGenerator> |
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 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.
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, 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.
@sarlinpe Mac is unhappy with the following error:
|
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 tofeatures/
.