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

Bug fixes and optimizations for sample consensus prerejective (replaces #736) #741

Merged
merged 7 commits into from
Jun 15, 2014
Merged

Bug fixes and optimizations for sample consensus prerejective (replaces #736) #741

merged 7 commits into from
Jun 15, 2014

Conversation

andersgb1
Copy link
Contributor

This PR solves a few bugs, and makes a number of significant improvements on both speed and robustness of the sample_consensus_prerejective class. It is simply a rehash of #736 with a clean up of the commits.

@taketwo
Copy link
Member

taketwo commented Jun 11, 2014

Somehow two extra commits managed to "leak" inside this pull request. You can remove them by doing interactive rebase against upstream master and then force pushing the branch to your fork.

@andersgb1
Copy link
Contributor Author

OK, hope I got it correct this time...

@VictorLamoine
Copy link
Contributor

Hey, here's a quick guide for you on how to edit your commits:
sudo rebase -i upstream/master

You will get the list of your commits

pick e68e087 Use a faster n-point sampler (thanks to Troels Bo Jørgensen for this code)
pick 87ab4b5 Remove a bug in a useless extra rejection step that caused a continue in all main loop iterations but the first (thanks to Jeppe Pedersen and Kent Hansen for catching this)
pick c458672 Use a cache for already computed feature correspondences (thanks to Jeppe Pedersen and Kent Hansen for the code)
pick e6c85df Avoid duplicate squared Euclidean distance computation
pick 103e0c2 Remove duplicate archive and use compressed PCDs to save space
pick 0738957 Explicitly specify the required PCL modules in the CMakeLists.txt file
pick 3350c0c Update the tutorial code to reflect the optimizations, leading to much faster and more robust pose estimation
pick bc91cae Put the parenthesis on a separate line

Replace "pick" by "e" if you want to edit a commit,
You can also squash commits etc.. as mentioned in the comments.
If you comment a line (with a # at the beginning), the commit will be destroyed.

When you are done save the file and you will get a message:

HEAD est maintenant sur bc91cae... Put the parenthesis on a separate line
dell@M6700:~/pcl$ git rebase -i upstream/master 
Stopped at e68e087d3956fef4c8e2bb4cd92041f206b4d57a... Use a faster n-point sampler (thanks to Troels Bo Jørgensen for this code)
You can amend the commit now, with

    git commit --amend 

Once you are satisfied with your changes, run

    git rebase --continue

You are now on the oldest commit you selected to edit. Make your changes to the files and do:
git add registration/* && git commit --amend && git rebase --continue
Change the commit message if necessary.

When you finish the rebasing you will get something like:

[HEAD détachée 0f10a0d] Use a faster n-point sampler (thanks to Troels Bo Jørgensen for this code)
 Author: Anders Glent Buch <andersgb1@gmail.com>
 2 files changed, 25 insertions(+), 19 deletions(-)
Successfully rebased and updated detached HEAD.

Now you can push your new modifications: (note that you must use a fast-forward push, -f)
git push -f origin

Don't hesitate if you have any question!

@andersgb1
Copy link
Contributor Author

Thanks @VictorLamoine, @taketwo . I should have made the changes now

@taketwo
Copy link
Member

taketwo commented Jun 15, 2014

Looks great now, thanks for the contribution!

taketwo added a commit that referenced this pull request Jun 15, 2014
Bug fixes and optimizations for sample consensus prerejective (replaces #736)
@taketwo taketwo merged commit 9dac3ec into PointCloudLibrary:master Jun 15, 2014
@andersgb1 andersgb1 deleted the sample_consensus_prerejective branch June 15, 2014 16:22
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.

3 participants