Skip to content
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

Fixed bug in correspondences of LS point to plane ICP and made norm(p… #6

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

stachnis
Copy link

Changes:

  1. Fixed bug in correspondences of LS point to plane ICP (in function: prepare_system_normals)
    Changes to:
    for (i, j) in correspondences:
    p_point = P[:, [i]]
    q_point = Q[:, [j]]
    nn = normals[j]
    e = nn.dot(error(x, p_point, q_point))
    J = nn.dot(jacobian(x, p_point))

  2. Compute and plot norm(P-Q) error consistently for all ICP variants for better comparison

…-q) plots being generated identical and look consistent over all ICP variants
@niosus
Copy link
Owner

niosus commented Apr 18, 2021

Thanks @stachnis!

I only now got to look at this and understand the change and you are totally right! The order of normals was, of course, totally messed up!

I merged the first change from a different PR (#7) as that generated a better diff. This PR adds 47 thousand lines! The diff might be so huge either because you did not re-run the notebook with a kernel restart before submitting or because it was done from a mac 🤷‍♂️

Could you tell me in some way what was the second change you suggested in this MR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants