Inconsistent behaviour of NaN treatment in `astropy.convolution.convolve` vs. `astropy.convolution.convolve_fft`

[cnavacch-spire]

Description

astropy.convolution.convolve returns different results depending on if NaN values are present in the input array or not. If no NaN value is present, then the result has a NaN buffer of kernel_size//2. If a NaN value is present, then the result is as expected and matches the output of astropy.convolution.convolve_fft.

Expected behavior

astropy.convolution.convolve should not have a NaN buffer of kernel_size//2 if no NaN value is present in the data. In addition the result should not differ depending on NaN values present in the data or not.

How to Reproduce

>>> import astropy.convolution
>>> from astropy.convolution import convolve, convolve_fft
>>> import numpy as np
>>> ar = np.ones((10, 10))
>>> ar
array([[1., 1., 1., 1., 1., 1., 1., 1., 1., 1.],
       [1., 1., 1., 1., 1., 1., 1., 1., 1., 1.],
       [1., 1., 1., 1., 1., 1., 1., 1., 1., 1.],
       [1., 1., 1., 1., 1., 1., 1., 1., 1., 1.],
       [1., 1., 1., 1., 1., 1., 1., 1., 1., 1.],
       [1., 1., 1., 1., 1., 1., 1., 1., 1., 1.],
       [1., 1., 1., 1., 1., 1., 1., 1., 1., 1.],
       [1., 1., 1., 1., 1., 1., 1., 1., 1., 1.],
       [1., 1., 1., 1., 1., 1., 1., 1., 1., 1.],
       [1., 1., 1., 1., 1., 1., 1., 1., 1., 1.]])
>>> gaussian = astropy.convolution.Gaussian2DKernel(1, x_size=5, y_size=5)
>>> convolve(ar, gaussian, boundary="fill", fill_value=np.nan, preserve_nan=True)
array([[nan, nan, nan, nan, nan, nan, nan, nan, nan, nan],
       [nan, nan, nan, nan, nan, nan, nan, nan, nan, nan],
       [nan, nan,  1.,  1.,  1.,  1.,  1.,  1., nan, nan],
       [nan, nan,  1.,  1.,  1.,  1.,  1.,  1., nan, nan],
       [nan, nan,  1.,  1.,  1.,  1.,  1.,  1., nan, nan],
       [nan, nan,  1.,  1.,  1.,  1.,  1.,  1., nan, nan],
       [nan, nan,  1.,  1.,  1.,  1.,  1.,  1., nan, nan],
       [nan, nan,  1.,  1.,  1.,  1.,  1.,  1., nan, nan],
       [nan, nan, nan, nan, nan, nan, nan, nan, nan, nan],
       [nan, nan, nan, nan, nan, nan, nan, nan, nan, nan]])
>>> convolve_fft(ar, gaussian, boundary="fill", fill_value=np.nan, preserve_nan=True)
array([[1., 1., 1., 1., 1., 1., 1., 1., 1., 1.],
       [1., 1., 1., 1., 1., 1., 1., 1., 1., 1.],
       [1., 1., 1., 1., 1., 1., 1., 1., 1., 1.],
       [1., 1., 1., 1., 1., 1., 1., 1., 1., 1.],
       [1., 1., 1., 1., 1., 1., 1., 1., 1., 1.],
       [1., 1., 1., 1., 1., 1., 1., 1., 1., 1.],
       [1., 1., 1., 1., 1., 1., 1., 1., 1., 1.],
       [1., 1., 1., 1., 1., 1., 1., 1., 1., 1.],
       [1., 1., 1., 1., 1., 1., 1., 1., 1., 1.],
       [1., 1., 1., 1., 1., 1., 1., 1., 1., 1.]])
>>> ar[5, 5] = np.nan
>>> convolve(ar, gaussian, boundary="fill", fill_value=np.nan, preserve_nan=True)
array([[ 1.,  1.,  1.,  1.,  1.,  1.,  1.,  1.,  1.,  1.],
       [ 1.,  1.,  1.,  1.,  1.,  1.,  1.,  1.,  1.,  1.],
       [ 1.,  1.,  1.,  1.,  1.,  1.,  1.,  1.,  1.,  1.],
       [ 1.,  1.,  1.,  1.,  1.,  1.,  1.,  1.,  1.,  1.],
       [ 1.,  1.,  1.,  1.,  1.,  1.,  1.,  1.,  1.,  1.],
       [ 1.,  1.,  1.,  1.,  1., nan,  1.,  1.,  1.,  1.],
       [ 1.,  1.,  1.,  1.,  1.,  1.,  1.,  1.,  1.,  1.],
       [ 1.,  1.,  1.,  1.,  1.,  1.,  1.,  1.,  1.,  1.],
       [ 1.,  1.,  1.,  1.,  1.,  1.,  1.,  1.,  1.,  1.],
       [ 1.,  1.,  1.,  1.,  1.,  1.,  1.,  1.,  1.,  1.]])

Versions

Linux-6.5.0-1020-aws-x86_64-with-glibc2.35
Python 3.10.12 | packaged by conda-forge | (main, Jun 23 2023, 22:40:32) [GCC 12.3.0]
astropy 6.1.0
Numpy 1.26.4
pyerfa 2.0.1.4
Scipy 1.13.0
Matplotlib 3.8.4

[github-actions[bot]]

Welcome to Astropy 👋 and thank you for your first issue!

A project member will respond to you as soon as possible; in the meantime, please double-check the guidelines for submitting issues and make sure you've provided the requested details.

GitHub issues in the Astropy repository are used to track bug reports and feature requests; If your issue poses a question about how to use Astropy, please instead raise your question in the Astropy Discourse user forum and close this issue.

If you feel that this issue has not been responded to in a timely manner, please send a message directly to the development mailing list. If the issue is urgent or sensitive in nature (e.g., a security vulnerability) please send an e-mail directly to the private e-mail feedback@astropy.org.

[pllim]

@keflavich , are you able to advise? Thanks!

[keflavich]

Yes, this looks like a bug in convolve; the convolve_fft behavior looks correct, and the convolve behavior is correct if a nan is present in the data originally. I think a fix is straightforward, either catching this case or removing the special-case for no NaNs in convolve.