pycpd icon indicating copy to clipboard operation
pycpd copied to clipboard

Adding correspondence priors to cpd

Open agporto opened this issue 2 years ago • 8 comments

In my pycpd fork, I've implemented the cpd extension described in https://ieeexplore.ieee.org/document/7477719 It allows users to add correspondence priors with varying degrees of confidence (from reliable to unreliable) to the usual cpd algorithm. It works both for the low rank as well as the standard cpd implementation. So, I thought I would send a pull request upstream in case there is broader interest in it. I've tested it in the context of biological research and it seems to be working well (no problems that I could observe). Let me know if you think it could be useful (or maybe it won't). In any case, here it goes. @gattia

agporto avatar May 19 '22 07:05 agporto

Hello @agporto!

Thank you for doing this. Can you also write an example that showcases the registration algorithm? I looked at the paper and it seems that they use this dataset: http://grail.cs.washington.edu/data/scans/

siavashk avatar Jun 06 '22 00:06 siavashk

Hello @agporto!

Thank you for doing this. Can you also write an example that showcases the registration algorithm? I looked at the paper and it seems that they use this dataset: http://grail.cs.washington.edu/data/scans/

Will do! I will update the pull request sometime in the next week or so.

agporto avatar Jun 07 '22 19:06 agporto

Hello @siavashk I've now updated the pull request with 3 examples (2D, 3D, lowrank) using the fish dataset that was already contained in the repo. In these examples, I simulate target pointclouds missing extensive parts. I checked out the dataset you suggested, but in the paper they use intrinsic shape descriptors to establish the initial correspondences. I did not want to add any extra dependency to pycpd, so the fish dataset seemed simpler. Please let me know what you think.

agporto avatar Jun 09 '22 18:06 agporto

This looks really good. I have to review it over the weekend but for now 👍

siavashk avatar Jun 09 '22 22:06 siavashk

@agporto Thanks for this! And sorry I've been checked out on this one so far. I'm still going to quickly review over the weekend, but in the meantime, do you have an objection if I switch this to be a pull-request off of the development branch instead? Im hoping to get a batch of these pull requests through to the development branch and then we can merge them all at once.

Also, is there a test you can think of for this? If so, do you mind adding it into the tests?

Thank you!

gattia avatar Jul 20 '22 20:07 gattia

@gattia No worries. I am happy to add some testing (probably this week). It will likely follow the pattern you have set. And yes, please feel free to switch the pull request to the development branch, but maybe you could wait until I update this pull request with testing (it is currently updating the pull request automatically from my branch). I will send you a message when I update the testing part.
Thanks!

agporto avatar Jul 26 '22 19:07 agporto

Thanks! That all sounds perfect.

gattia avatar Jul 27 '22 03:07 gattia

Hi @gattia. The McKinney fire caused me to delay it a bit, but here is the testing file. Please let me know if there is anything that is not clear. Please feel free to change it to the development branch at this point. Thanks!

agporto avatar Aug 09 '22 19:08 agporto

@agporto - Im very sorry for the long delay on this. It's taken me a while to sit down and look it all over. This looks great. I've made a few tweaks and pushed them to your repository.

  1. removed some redundancy in creating the gaussian kernel etc. (https://github.com/siavashk/pycpd/pull/57/commits/305b6fdcd1c275dcaa00b0b808541835b9768658)
  2. updated the examples to include coloring that identifies what points are constraining the registration. (https://github.com/siavashk/pycpd/pull/57/commits/5a8015ae2b960c409494050c8beb269d7fafcdb5)

Let me know what you think about these. If you're OK with them, then I'll proceed to merging them with the development branch and then master.

gattia avatar Nov 29 '22 04:11 gattia

@gattia Looks great. Makes sense to remove the redundancy. And the identification of the points constraining registration is very useful. So, please feel free to proceed with merging. There are a few conflicts with main branch to resolve. One is essentially due to the redundancy removal. The other is related to the "is not" syntax in the main repo (which I substituted in my repo for !=). I will leave those conflicts for you to resolve, if that is ok with you.

agporto avatar Dec 02 '22 21:12 agporto

@agporto - that's fine, I'll move to development and then fix everything over there. Thanks for this contribution!

gattia avatar Dec 02 '22 21:12 gattia

@agporto - I made one final set of changes where I had your new class inherit from the base deformable_registration class to reduce some redundancy and let the new class gain any bug fixes or other changes that the base class receives.

Thank you so much for this addition! It was clearly a lot of work and is an excellent contribution to the library.

Best,

Anthony

gattia avatar Dec 07 '22 21:12 gattia

@gattia It was my pleasure. Thanks to you and @siavashk for pycpd in the first place. It has been very helpful in my research.

agporto avatar Dec 26 '22 22:12 agporto