-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Phase cross correlation is not a phase cross correlation #5459
Comments
Thanks for reporting this @JulesScholler! Indeed it seems this is a long-standing bug (that appears to originate in the Matlab-central implementation mentioned in the comment at the top of the file). Here is a simple illustration of the effect of the proposed change on a small example: import matplotlib.pyplot as plt
import numpy as np
from scipy import fft
from skimage import data, img_as_float
src = img_as_float(data.camera()[128:256, 128:256])
src = src + 0.05 * np.random.standard_normal(src.shape)
target = np.roll(src, (15, -10), axis=(0, 1))
target = target + 0.05 * np.random.standard_normal(target.shape)
src_freq = fft.fftn(src)
target_freq = fft.fftn(target)
# current implementation
image_product1 = src_freq * target_freq.conj()
cross_correlation1 = fft.ifftn(image_product1)
# fixed implementation
image_product = image_product1 / np.abs(image_product1)
cross_correlation = fft.ifftn(image_product)
fig, axes = plt.subplots(1, 2)
axes[0].imshow(np.abs(cross_correlation1), cmap=plt.cm.gray)
axes[0].set_title('Existing Implementation')
axes[1].imshow(np.abs(cross_correlation), cmap=plt.cm.gray)
axes[1].set_title('Proposed Implementation')
for ax in axes:
ax.set_axis_off()
plt.tight_layout()
plt.show() In the figure below, the left image is the magnitude of the |
Do you happen to have a test case that would succeed with this change, but currently fails without it? (Our existing test suite passes both for the existing implementation as well as with this proposed modification) |
With slight modification to avoid potential divide by zero: eps = np.finfo(image_product.real.dtype).eps
image_product /= (np.abs(image_product) + eps) this fix resolves the case brought up in #5460 as well |
Hello @grlee77! Great that you added a regularization to avoid dividing by zero. Did you manage to find a case where it would fail with the normal cross correlation and not with the phase cross correlation? Otherwise I suggest to occult part of the moving image by adding a black/white disc or square. Let me know if you want me to test some things! |
Could it be this broke the RMS error return value? #7078 |
scikit-image/skimage/registration/_phase_cross_correlation.py
Lines 118 to 288 in 2bbfce4
For this function to compute the phase cross correlation the
image_product
should be divided bynp.abs(image_product)
line 220. Also theoverlap_ratio
is not used (so it doesn't actually work as intended) and thus very confusing.The phase cross correlation is explained here in detail: https://en.wikipedia.org/wiki/Phase_correlation
The text was updated successfully, but these errors were encountered: