Skip to content
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

exact align should support a tolerance for agreement in coordinates #5094

Closed
bennyrowland opened this issue Mar 30, 2021 · 2 comments
Closed

Comments

@bennyrowland
Copy link

What happened:
Using xrft to do 2D Fourier transforms of DataArrays, the coords for transformed axes are modified along with the data. If we do both a forward and inverse transformation, the values are fractionally different to those from the original data, which causes align(join="exact") to fail, and in turn things which depend on this such as where(). This does not seem to happen for a 1D transform, but does for a 2D, for the outer axis. If we use np.allclose() then the coords are shown to be essentially equal to an extremely high tolerance.

What you expected to happen:
align(join="exact") should use np.allclose() or some alternative approach which allows inconsequential differences between coords. This could be supported with a keyword argument to align() with some kind of tolerance to be passed to np.allclose() with a conservative default value to allow xr.where() etc. to succeed without having to call align() manually first.

Minimal Complete Verifiable Example:

import numpy as np
import xarray as xr
import xrft.xrft as xrft

data = np.random.random((150, 150))
coords1 = np.fft.fftshift(np.fft.fftfreq(150, 200/150))
da = xr.DataArray(data, dims=["d1", "d2"], coords={"d1": coords1, "d2": coords1})

fda = xrft.idft(da)
ff_da = xrft.dft(fda)

print(np.allclose(da.d1, ff_da.d1, rtol=1e-14, atol=1e-16))
xr.align(da, ff_da, join='exact')

Anything else we need to know?:
I am happy to work on a PR if the core devs can confirm they would approve the change.

Environment:

Output of xr.show_versions()

INSTALLED VERSIONS

commit: None
python: 3.8.8 (default, Feb 24 2021, 15:54:32) [MSC v.1928 64 bit (AMD64)]
python-bits: 64
OS: Windows
OS-release: 10
machine: AMD64
processor: Intel64 Family 6 Model 142 Stepping 12, GenuineIntel
byteorder: little
LC_ALL: None
LANG: None
LOCALE: English_United Kingdom.1252
libhdf5: 1.10.4
libnetcdf: None

xarray: 0.17.0
pandas: 1.2.3
numpy: 1.19.2
scipy: 1.6.1
netCDF4: None
pydap: None
h5netcdf: None
h5py: 2.10.0
Nio: None
zarr: None
cftime: 1.4.1
nc_time_axis: None
PseudoNetCDF: None
rasterio: None
cfgrib: None
iris: None
bottleneck: 1.3.2
dask: 2021.03.0
distributed: 2021.03.0
matplotlib: 3.3.4
cartopy: None
seaborn: None
numbagg: None
pint: None
setuptools: 52.0.0.post20210125
pip: 21.0.1
conda: None
pytest: 6.2.2
IPython: 7.21.0
sphinx: 3.5.2

@keewis
Copy link
Collaborator

keewis commented Mar 31, 2021

related to #4489

@bennyrowland
Copy link
Author

Indeed, it looks like that will solve my problem when it gets merged. I did a search for existing issues but missed 4489 as a PR. I will close this and keep an eye on 4489 instead. Thanks for bringing it to my attention.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants