-
Notifications
You must be signed in to change notification settings - Fork 769
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
Switching to squared sampson point-line error #792
Conversation
Wait, is this a WIP PR? @akshay-krishnan @ayushbaid ? |
Also this is a breaking PR. |
Fan, yes this a WIP PR. We merged a portion of it in another PR. I am not sure how Ayush wants to proceed with this, I think we will open a new PR if we want to merge the remaining changes. |
Yeah what I mean is that this will change how old code behaves or not? |
It seems this was merged in develop, not in another branch, so Fan questions are pertinent :-) Phan, why do you say this is a breaking PR? CI seems to work. |
@dellaert Because double EssentialMatrix::error(const Vector3& vA, const Vector3& vB,
OptionalJacobian<1, 5> HE,
OptionalJacobian<1, 3> HvA,
OptionalJacobian<1, 3> HvB) const { signature has changed, and
The first Jacobian has changed as well. |
Fan, those changes have not been merged into develop. Here is the error() method in EssentialMatrix.cpp at develop. So git will say that this PR has been merged since its commits went into #753 but I removed most of the changes in those commits later on. |
@akshay-krishnan I see, thanks :) |
The squared sampson error is the first order approximation of the gold-standard reprojection error.
This is a work-in-progress PR, and I'll see if the switch actually improves performance. I will clean up the PR and add more documentation if we want to merge this.