-
Notifications
You must be signed in to change notification settings - Fork 855
LOST Triangulation Underconstrained #1534
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
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.
Thank you for catching this!
On the gtsam repo, it is actually not encouraged to format entire files. Its also hard for reviewers to spot the relevant change. Other than setting the rank tolerance for Eigen, did I miss anything?
If you have a small example with only 2 cameras where this happens, it might be worth adding a unit test too.
The issue was due to the baseline vector coinciding with the jth measurement vector ( |
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.
could you revert the formatting changes from the first commit?
gtsam/geometry/triangulation.cpp
Outdated
double num_i = wZi.cross(wZj).norm(); | ||
double den_i = d_ij.cross(wZj).norm(); | ||
|
||
// Handle q_i = 0 (or NaN), which arises if the measurement vectors, wZi and wZj, coincide |
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.
is this line exceeding the max line length?
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.
Yes, fixed
Reverted all of the formatting. |
Hi @travisdriver. Sorry for the extra work, but can you please add a unit test as well. You can copy-paste the post-processed values from the Hayabusa mission to serve as your test fixture. This will allow us to maximize code coverage and guarantee correctness in the future as well. |
I'm going to merge - a unit test would be nice but we have those in GTSFM, and the code does have a set of unit tests already |
I encountered an instance where the iterated DLT solution would correctly triangulate an input point, while the LOST algorithm would return NaNs.
This seems to be related to a rank deficiency in the QR factorization. The rank tolerance was being set for the DLT methods, but it was not being set for LOST and was instead being computed automatically by Eigen.
Therefore, I added the
rank_tol
variable to thetriangulateLOST
method so that the user could explicitly set the tolerance. However, this change didn't seem to fix the rank deficiency issue.Below is code to reproduce the issue. The measurements and camera data were derived from images and ancillary data from the Hayabusa mission to Asteroid 25143 Itokawa.
Data in COLMAP format:
lost_underconstrained_data.zip
The
read_write_model
module is defined here.