Added in wrap_axes to cross_correlation_masked function

[CSSFrancis]

Description

This PR adds in the ability to define which axes should be padded and which should wrap for the cross_correlation_masked function.

Specifically this is useful for polar unwrapped images where (some) edges of image should be wrapped to determine orientation and registration.

Example:

import numpy as np
import matplotlib.pyplot as plt
from skimage.registration._masked_phase_cross_correlation import cross_correlate_masked

test = np.zeros((10, 20))
test[:,0] = 100
test[:,10] = 300

test_rot= np.zeros((10, 20))
test_rot[:,4] = 100
test_rot[:,14] = 300

wrapped_cor = cross_correlate_masked(test, test_rot,
                                  m1 = np.ones((10,20)),
                                  m2= np.ones((10,20)),
                                  axes=(1,),
                                  wrap_axes=(1,)).real)
no_wrap_cor = cross_correlate_masked(test, test_rot,
                                  m1 = np.ones((10,20)),
                                  m2= np.ones((10,20)),
                                  axes=(1,),mode="same").real)

Checklist

• Docstrings for all functions
• Gallery example in ./doc/examples (new features only)
• Benchmark in ./benchmarks, if your changes aren't covered by an
  existing benchmark
• Unit tests
• Clean style in the spirit of PEP8
• Descriptive commit messages (see below)

For reviewers

• Check that the PR title is short, concise, and will make sense 1 year
  later.
• Check that new functions are imported in corresponding __init__.py.
• Check that new features, API changes, and deprecations are mentioned in
  doc/release/release_dev.rst.

[pep8speaks]

Hello @CSSFrancis! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-09-24 15:56:20 UTC

[grlee77]

Thanks, @CSSFrancis, this change looks reasonable to me.

Based on the linked pyxem/pyxem#750 issue, it looks like you want to rely on this function in a downstream package? That is great, but to make that more robust, we should also consider officially adding this function to our public API. Notice that it is not currently among the functions listed in skimage/registration/__init__.py and doesn't show up in the online API docs. It is used by the public phase_cross_correlation function, so it is unlikely to be removed, but adding to the public API would guarantee that the import path or arguments would not change abruptly without a deprecation cycle.

[CSSFrancis]

@grlee77 Thanks for the notes!

Based on the linked pyxem/pyxem#750 issue, it looks like you want to rely on this function in a downstream package?

Yes, there are a couple of instances working with polar coordinates where this might be very useful for us.

That is great, but to make that more robust, we should also consider officially adding this function to our public API. Notice that it is not currently among the functions listed in skimage/registration/__init__.py and doesn't show up in the online API docs. It is used by the public phase_cross_correlation function, so it is unlikely to be removed, but adding to the public API would guarantee that the import path or arguments would not change abruptly without a deprecation cycle.

I can add in an example into the doc string and then add it to skimage/registration/__init__.py.

Looking at this a bit more closely, it might be useful to add in the option for a wrap_axis to the phase_cross_correlation function. Currently, I think that the polar example for phase_cross_correlation. works very well because there is one strong feature in the image. If there were a couple of strong features in the image I think there could be some odd behavior (especially if the features are more than 180 degrees separated) due to how the function deals with masked values. I have shown a little of this behavior in the 1D example above.

This is kind of an edge case, but I think that wrapping the polar axis would be more correct in a polar example than the current implementation.

[jni]

@grlee77 @CSSFrancis Thanks for this improvement! But, imho, we should keep the public API the same, accessed through skimage.registration.phase_cross_correlation. So I agree with your assessment @CSSFrancis that we should add wrap_axis to phase_cross_correlation and pass through to your improvements here.

[CSSFrancis]

Coming back to this I think that there are a couple of things that we could potentially improve on with regard to the phase_cross_correlate function. I would be willing to open a separate PR and introduce a depreciation cycle.

There are a couple of inconsistencies between the masked and unmasked versions which would be easy to rectify

• The default for unmasked versions is to wrap some axis (https://docs.scipy.org/doc/scipy/reference/generated/scipy.fft.fftn.html) while the default for the masked version is to pad the data before the fft

  scikit-image/skimage/registration/_masked_phase_cross_correlation.py

  Line 199 in fa43626

  fft = partial(fftmodule.fftn, s=fast_shape, axes=axes)

• Masked versions cannot be upsampled, this could be easily added
• Normalization is only used for the unmasked version.

Additionally there an easy change that could be implemented and might be of interest would be applying the cross-correlation to only certain axes

[grlee77]

Coming back to this I think that there are a couple of things that we could potentially improve on with regard to the phase_cross_correlate function. I would be willing to open a separate PR and introduce a depreciation cycle.

I'm definitely in favor of making things as consistent as possible across masked and unmasked cases. We are currently in the process of getting 0.19 out the door and have a list of quite a few things we want to clean up in the API after that. This seems in line with those goals.

[jni]

@CSSFrancis awesome suggestions! 👍

[alexdesiqueira]

Hey everyone,
how are we on this one — and #6077, also?
Thank you for all your work so far, @CSSFrancis! Let us know if there's anything we (@grlee77 @jni also) could help you with.
Thanks again!

[CSSFrancis]

Sorry for the slow response. Just hit the end of the semester so I'm only now getting time to come back to this. I think that we can close this in favor of #6077.