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

window_correction seems sensitive to formatting of dim #142

Closed
cspencerjones opened this issue Mar 18, 2021 · 12 comments · Fixed by #143
Closed

window_correction seems sensitive to formatting of dim #142

cspencerjones opened this issue Mar 18, 2021 · 12 comments · Fixed by #143

Comments

@cspencerjones
Copy link

I haven't been able to reproduce this with synthetic data (sorry!)

But for my real data, xrft.power_spectrum(u_unfiltered.isel(x0=1000,y0=1000).load(),dim='time', window='hann',window_correction=True) gives the error:

---------------------------------------------------------------------------
KeyError                                  Traceback (most recent call last)
~/.conda/envs/pangeo2/lib/python3.7/site-packages/xarray/core/dataarray.py in _getitem_coord(self, key)
    692         try:
--> 693             var = self._coords[key]
    694         except KeyError:

KeyError: 't'

During handling of the above exception, another exception occurred:

KeyError                                  Traceback (most recent call last)
<ipython-input-40-09826e26c18a> in <module>
----> 1 xrft.power_spectrum(u_unfiltered.isel(x0=1000,y0=1000).load(),dim='time', window='hann',window_correction=True)

~/.conda/envs/pangeo2/lib/python3.7/site-packages/xrft/xrft.py in power_spectrum(da, dim, real_dim, scaling, window_correction, **kwargs)
    712                 )
    713             else:
--> 714                 windows, _ = _apply_window(da, dim, window_type=kwargs.get("window"))
    715                 ps = ps / (windows ** 2).mean()
    716         fs = np.prod([float(ps[d].spacing) for d in updated_dims])

~/.conda/envs/pangeo2/lib/python3.7/site-packages/xrft/xrft.py in _apply_window(da, dims, window_type)
     97             win_func(len(da[d]), sym=False), dims=da[d].dims, coords=da[d].coords
     98         )
---> 99         for d in dims
    100     ]
    101 

~/.conda/envs/pangeo2/lib/python3.7/site-packages/xrft/xrft.py in <listcomp>(.0)
     97             win_func(len(da[d]), sym=False), dims=da[d].dims, coords=da[d].coords
     98         )
---> 99         for d in dims
    100     ]
    101 

~/.conda/envs/pangeo2/lib/python3.7/site-packages/xarray/core/dataarray.py in __getitem__(self, key)
    702     def __getitem__(self, key: Any) -> "DataArray":
    703         if isinstance(key, str):
--> 704             return self._getitem_coord(key)
    705         else:
    706             # xarray-style array indexing

~/.conda/envs/pangeo2/lib/python3.7/site-packages/xarray/core/dataarray.py in _getitem_coord(self, key)
    695             dim_sizes = dict(zip(self.dims, self.shape))
    696             _, key, var = _get_virtual_variable(
--> 697                 self._coords, key, self._level_coords, dim_sizes
    698             )
    699 

~/.conda/envs/pangeo2/lib/python3.7/site-packages/xarray/core/dataset.py in _get_virtual_variable(variables, key, level_vars, dim_sizes)
    169         ref_var = dim_var.to_index_variable().get_level_variable(ref_name)
    170     else:
--> 171         ref_var = variables[ref_name]
    172 
    173     if var_name is None:

KeyError: 't'

Whereas xrft.power_spectrum(u_unfiltered.isel(x0=1000,y0=1000).load(),dim=['time'], window='hann',window_correction=True) is totally fine.

Any idea what is causing this? Is all testing performed on data with single character dimension names? Perhaps that is the problem?

@roxyboy
Copy link
Member

roxyboy commented Mar 18, 2021

Do you mean that if you generate a synthetic data with the same dimensions and coordinates as your u_unfiltered, you don't get an error whether dim is a string or list?
And one naive question: why are you loading the data before applying xrft...?

@cspencerjones
Copy link
Author

And one naive question: why are you loading the data before applying xrft...?

Oh sorry I think that was left in from debugging!

Do you mean that if you generate a synthetic data with the same dimensions and coordinates as your u_unfiltered, you don't get an error whether dim is a string or list?

No this is not what I mean. I just tried with the example from the docs in my setup and I didn't get the error. But the example has dimension name 'x', and I haven't tried tweaking it.

@dougiesquire
Copy link
Contributor

dougiesquire commented Mar 18, 2021

It looks like _apply_window does not support dim being a string (only a list of string). We must've missed this, but it should be an easy fix.

@dougiesquire
Copy link
Contributor

dougiesquire commented Mar 18, 2021

The issue is that there is a for d in dims inside _apply_window, so yes, things will work for single character dimension names

@roxyboy
Copy link
Member

roxyboy commented Mar 18, 2021

I think in dft, if dim is given as a string, it is converted to a list...?

if isinstance(dim, str):

So the dim that _apply_window takes is a list no matter what, I think.

@dougiesquire
Copy link
Contributor

I think in dft, if dim is given as a string, it is converted to a list...?

if isinstance(dim, str):

So the dim that _apply_window takes is a list no matter what, I think.

Hmm weird... I'll try and get to the bottom of this this morning

@rabernat
Copy link
Collaborator

Thanks for reporting this @cspencerjones! We appreciate it.

@paigem
Copy link
Contributor

paigem commented Mar 23, 2021

I think I've just run into this same problem when using xrft's detrend() function. The code I tried to run is included below, but it looks like the same fix that @dougiesquire made in _apply_window() should be added to detrend() as well. I can probably make the fix by the end of today.

I'm using xrft version 0.3.0 on Pangeo Cloud.

#Create dummy data
import numpy as np
x = np.arange(-5,5,0.01)
a = np.sin(2*x) + x + 3

# Convert to xarray
import xarray as xr
ax = xr.DataArray(a,dims=['time'])

# No error with constant detrend
import xrft
xrft.detrend(ax,'time',detrend_type='constant')

# Error with linear detrend
xrft.detrend(ax,'time',detrend_type='linear')
ValueError --------------------------------------------------------------------------- ValueError Traceback (most recent call last) /srv/conda/envs/notebook/lib/python3.8/site-packages/xarray/core/common.py in _get_axis_num(self, dim) 171 try: --> 172 return self.dims.index(dim) 173 except ValueError:

ValueError: tuple.index(x): x not in tuple

During handling of the above exception, another exception occurred:

ValueError Traceback (most recent call last)
in
----> 1 ax_detrend_linear = xrft.detrend(ax2,'time',detrend_type='linear')

/srv/conda/envs/notebook/lib/python3.8/site-packages/xrft/detrend.py in detrend(da, dim, detrend_type)
49 elif detrend_type == "linear":
50 data = da.data
---> 51 axis_num = [da.get_axis_num(d) for d in dim]
52 chunks = getattr(data, "chunks", None)
53 if chunks:

/srv/conda/envs/notebook/lib/python3.8/site-packages/xrft/detrend.py in (.0)
49 elif detrend_type == "linear":
50 data = da.data
---> 51 axis_num = [da.get_axis_num(d) for d in dim]
52 chunks = getattr(data, "chunks", None)
53 if chunks:

/srv/conda/envs/notebook/lib/python3.8/site-packages/xarray/core/common.py in get_axis_num(self, dim)
166 return tuple(self._get_axis_num(d) for d in dim)
167 else:
--> 168 return self._get_axis_num(dim)
169
170 def _get_axis_num(self: Any, dim: Hashable) -> int:

/srv/conda/envs/notebook/lib/python3.8/site-packages/xarray/core/common.py in _get_axis_num(self, dim)
172 return self.dims.index(dim)
173 except ValueError:
--> 174 raise ValueError(f"{dim!r} not found in array dimensions {self.dims!r}")
175
176 @Property

ValueError: 't' not found in array dimensions ('time',)

@rabernat
Copy link
Collaborator

One possible workaround for this issue is to just put the dim inside a list, e.g.

xrft.detrend(ax, ['time'], detrend_type='constant')

Does this work?

@paigem
Copy link
Contributor

paigem commented Mar 23, 2021

@rabernat yep I did that so I could run my current computation, but I still think it probably makes sense to update detrend() to allow for the dim to just be a string.

@dougiesquire
Copy link
Contributor

Thanks for bringing this up @paigem. I'll also update detrend() in the current PR (#143).

@paigem
Copy link
Contributor

paigem commented Mar 23, 2021

That would be great, thanks @dougiesquire!

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

Successfully merging a pull request may close this issue.

5 participants