Documentation + self.w unused

siavashk / pycpd

Pure Numpy Implementation of the Coherent Point Drift Algorithm

MIT License

513 stars 115 forks source link

Documentation + self.w unused #12

Closed normanius closed 6 years ago

normanius commented 6 years ago

Hi Siavash

Just a few details:

would be great if there was some bit of code documentation (mainly constructor arguments and returned objects)
the self.w does not have any effect on the result

In any case great work! Thanks a lot for your efforts. Cheers, Norman

siavashk commented 6 years ago

Hi Norman,

Thank you for the code review. I am always happy when I receive them. Please do tell me if you find anything else. Regarding your two points:

Documentation: I would love to, but I just haven’t gotten into that. My day job keeps me pretty buys. Sorry about that!
self.w is the contribution of the uniform distribution to mixture model. It is used in the EStep function of each registration class. See equations (2) and (6) in Myrorenko and Song’s paper:

We also added an additional uniform distribution to the mixture model to account for noise and outliers. Denoting the weight of the uniform distribution as w...

normanius commented 6 years ago

No worries regarding the documentation. Would have been handy though.

Regarding the second point: I see that self.w (lowercase w) is used in Estep() to compute the variable lowercase c. However c is never used, so self.w has no effect in the end. I am referring to deformable_registration().

Do I oversee something?

siavashk commented 6 years ago

Good catch. I forgot to add it to the denominator. Should probably go here.

siavashk commented 6 years ago

Fixed in 3a22216b5d8bf3e035ec06afab6d33862be2021e