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

groupby_bins groups not correctly applied with built-in methods #7601

Closed
4 tasks done
michaelaye opened this issue Mar 9, 2023 · 3 comments · Fixed by #7206
Closed
4 tasks done

groupby_bins groups not correctly applied with built-in methods #7601

michaelaye opened this issue Mar 9, 2023 · 3 comments · Fixed by #7206

Comments

@michaelaye
Copy link

michaelaye commented Mar 9, 2023

What happened?

Setup

I want to calculate image statistics per chunk in one dimension. Let's assume a very small image for demonstration purposes:

a = xr.DataArray(np.arange(12).reshape(6,2), dims=('x', 'y'))
a

image
Trying to chunk this into three subimages, I use these bins into the x dimension:

x_bins = (0, 2, 4, 6)

I look at the groups this creates by default:

for iv, g in a.groupby_bins('x', x_bins):
    print(iv)
    print(g)

image
I don't understand the use-case for this grouping, as it's missing the beginning and is having uneven sized last group (Obviously a follow-error from not including the first row).

To force the even chunking of the image I need to call it with these parameters:

groups = a.groupby_bins('x', x_bins, include_lowest=True, right=False)
for iv, g in groups:
    print(iv)
    print(g)

image

Issue

But now, calculating the mean value of each group, I get different results when doing it by hand using the groups or doing it using the groups inherent method mean():

image

Indeed, I verified, that these results are what one gets, using the first version of applying the bins:
image

The same is true when I use the elliptical operator to receive the mean over the remaining dimensions (note, the 2nd cell here is using the groups variable as defined in the cell before, so should really return the same values, but it doesn't:
image

Application

I believe that groupby_bins is the most appropriate tool to do this in xarray. I wished that one could enforce the dask-chunks in dask arrays to survive and return stats from them, but haven't found a way to do that.

What did you expect to happen?

That the inherent stats methods of the groups method respect the interval use constraints from the groupby_bins call.

I also have verified that the same problem exists with groups.std().

Minimal Complete Verifiable Example

import xarray as xr
import numpy as np

a = xr.DataArray(np.arange(12).reshape(6,2), dims=('x', 'y'))

x_bins = (0, 2, 4, 6)

default_groups = a.groupby_bins('x', x_bins)
my_groups = a.groupby_bins('x', x_bins, include_lowest=True, right=False)

print("Weird grouping using default call:")
for iv, g in default_groups:
    print("Interval:",iv)
    print(g.data)
    print()
    
print("Evenly chunked using `my_groups`:")    
for iv, g in my_groups:
    print("Interval:", iv)
    print(g.data)
    print()

print("Calculating mean on my own using loop over groups:")
for iv, g in my_groups:
    print(g.mean('x').data)
    
print("Calculting same using my_groups.mean()")
print("No dim given:")
print(my_groups.mean().data.T)
print("using mean('x'):")
print(my_groups.mean('x').data.T)

print("These results come from the default groups!:")
for iv, g in default_groups:
    print(g.mean('x').data)
    
print("STD has the same issue")

MVCE confirmation

  • Minimal example — the example is as focused as reasonably possible to demonstrate the underlying issue in xarray.
  • Complete example — the example is self-contained, including all data and the text of any traceback.
  • Verifiable example — the example copy & pastes into an IPython prompt or Binder notebook, returning the result.
  • New issue — a search of GitHub Issues suggests this is not a duplicate.

Relevant log output

No response

Anything else we need to know?

No response

Environment

INSTALLED VERSIONS

commit: None
python: 3.10.9 | packaged by conda-forge | (main, Feb 2 2023, 20:20:04) [GCC 11.3.0]
python-bits: 64
OS: Linux
OS-release: 6.0.12-76060006-generic
machine: x86_64
processor: x86_64
byteorder: little
LC_ALL: en_US.UTF-8
LANG: en_US.UTF-8
LOCALE: ('en_US', 'UTF-8')
libhdf5: 1.12.2
libnetcdf: 4.9.1

xarray: 2023.2.0
pandas: 1.5.3
numpy: 1.23.5
scipy: 1.10.1
netCDF4: 1.6.3
pydap: None
h5netcdf: None
h5py: 3.8.0
Nio: None
zarr: None
cftime: 1.6.2
nc_time_axis: None
PseudoNetCDF: None
rasterio: 1.3.6
cfgrib: None
iris: None
bottleneck: 1.3.7
dask: 2023.3.0
distributed: 2023.3.0
matplotlib: 3.7.1
cartopy: 0.21.1
seaborn: 0.12.2
numbagg: None
fsspec: 2023.3.0
cupy: None
pint: None
sparse: None
flox: 0.6.8
numpy_groupies: 0.9.20
setuptools: 67.5.1
pip: 23.0.1
conda: installed
pytest: 7.1.3
mypy: None
IPython: 8.7.0
sphinx: None

@michaelaye michaelaye added bug needs triage Issue that has not been reviewed by xarray team member labels Mar 9, 2023
@dcherian
Copy link
Contributor

dcherian commented Mar 9, 2023

I think this is a bug with not passing cut_kwargs to flox.

Use xr.set_options(use_flox=False) for now. I think this is fixed in #7206

@dcherian dcherian added topic-groupby and removed needs triage Issue that has not been reviewed by xarray team member labels Mar 9, 2023
@dcherian
Copy link
Contributor

dcherian commented Mar 9, 2023

I wished that one could enforce the dask-chunks in dask arrays to survive and return stats from them, but haven't found a way to do that

What do you mean?

dcherian added a commit to dcherian/xarray that referenced this issue Mar 9, 2023
@michaelaye
Copy link
Author

That, while I can open an xarray as a dask array using chunks:

image

I cannot make "use" of these chunks to get statistics per chunk, right? It's just an efficiency question for the dask.compute() stage, but not an actual way to get statistics per chunk?

dcherian added a commit that referenced this issue Mar 29, 2023
* Save groupby codes after factorizing

* Fix tests

* Fix cftime resampling

* Fix resampling codes.

* codes is always a DataArray.

* cleanup

* WIP

* Revert "WIP"

This reverts commit c9e2a62.

* Fix doctests

* Fix cftime resampling

Fix cftime resampling

* Better resampling example code.

* Fix test.

* Fix bad merge

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Try fixing some typing

* Avoid warning

* Fix typing

* Add regression  test for #7601

Closes #7601

* Cleanup + fix typnig

* Typing run

* Fix

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Illviljan <14371165+Illviljan@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants