-
Notifications
You must be signed in to change notification settings - Fork 51
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
Using tracks in translation averaging #594
Conversation
All places where we see a negative impact are already small numbers in terms of absolute value. |
Latency for translation averaging is expected to increase by 2-3x. This is dominated by an increase in the outlier rejection time as we have more measurements. |
def __get_initial_values(self, wTi_initial: List[Optional[PosePrior]]): | ||
# Compute outlier weights using MFAS. | ||
# TODO(ayush): parallelize this step. | ||
outlier_weights: List[Dict[Tuple[int, int], float]] = [] |
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.
Just curious: can we make outlier_weights dict keyed on symbols instead of integers?
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.
it is already. I dont know why there is an int type hint. are Symbols ints in python?
for (i1, i2), i2Ui1 in i2Ui1_dict.items(): | ||
wRi2 = wRi_list[i2] | ||
if i2Ui1 is not None and wRi2 is not None: | ||
# TODO: what if wRi2 is None, but wRi1 is not? Can we still transform. |
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.
Why are we enforcing both cameras to have a global rotation. It makes sense to do it, but why do it now?
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 except few comments
Merging this, I think all comments have been addressed, thanks for the review! |
We have the 2D tracks before translation averaging. These essentially give us 3D direction measurements from cameras to 3D points. Following 1DSfM, we add these to the translation averaging problem to provide more constraints.
We only add a few tracks (this number needs to be tuned).
This PR also refactors translation averaging, moves some functions to utils, and adds unit tests.