siavashk / pycpd

Pure Numpy Implementation of the Coherent Point Drift Algorithm
MIT License
522 stars 118 forks source link

Division by zero #66

Closed mxf8bv closed 1 year ago

mxf8bv commented 2 years ago

Hi,

the parameter w is not checked for range, so this line: c = c * self.w / (1 - self.w)

can divide by zero. How about an additional line in init: assert self.w<=0<1

BTW: I find it hard to understand the meaning of the documentation of w without reading the original paper. I suppose w=0 (default) is the case for Gaussian distributions only and w=1 for equal distribution (in the presence of noise), right?

siavashk commented 2 years ago

Thanks for catching the potential bug. I am kind of overwhelmed by other responsibilities but would like to fix this sometime in the future.

I would prefer raising a ValueError instead of an assert.

w is a measure of outliers, i.e. target points that do not belong to the target point cloud but exist due to noise in measurements. The paper makes the assumption that outliers can appear anywhere, hence they are modeled as a uniform distribution. w=0 means that there are no outliers in the data. w≈1.0 means that the entire point cloud is outliers.

gattia commented 1 year ago

Im closing this because the current code already throws a ValueError if w >= 1 therefore this case shouldn't happen.

See: https://github.com/siavashk/pycpd/blob/83ffd5ae14ec898f4415f34ed6fd183c34026804/pycpd/emregistration.py#L163-L165