-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Compute residuals with threadpool for model_merger #3401
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
base: main
Are you sure you want to change the base?
Conversation
src/colmap/estimators/alignment.cc
Outdated
const Image& src_image = *src_images[i]; | ||
const Image& tgt_image = *tgt_images[i]; | ||
if (thread_pool_ && src_images.size() > 10) { | ||
std::vector<std::future<void>> futures; |
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.
No need to accumulate the futures. You can just do thread_pool_->Wait(); to finish all tasks in the pool.
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.
Fixed but does not propagate THROW_CHECK_EQ
exceptions. How could this be solved?
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 removed THROW_CHECK_EQ
from ComputeImageResidual
, and only perform the checks once before computing the residuals.
src/colmap/sfm/observation_manager.h
Outdated
@@ -33,13 +33,15 @@ | |||
#include "colmap/scene/image.h" | |||
#include "colmap/scene/reconstruction.h" | |||
#include "colmap/scene/track.h" | |||
#include "colmap/util/threading.h" |
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.
Let's forward-decl threadpool here as well instead of including this header.
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.
Fixed
Hey @ahojnnes ,
We've recently had a computation bottleneck when trying to merge big reconstructions (2,500 to 35,000 common images). The heaviest step is computing the residuals for alignment. This PR simply computes the residuals with a threadpool.
On two models with 2,706 common images the alignment step drops from 646 s to 51 s with 64 threads.
N.B.:
THROW_CHECK_EQ
exceptions in threads are propagated in the main thread throughfuture.get()
.