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

Lazy MEAN Aggregator doesn't support weights #3129

Closed
DPeterK opened this issue Aug 3, 2018 · 3 comments · Fixed by #3299
Closed

Lazy MEAN Aggregator doesn't support weights #3129

DPeterK opened this issue Aug 3, 2018 · 3 comments · Fixed by #3299

Comments

@DPeterK
Copy link
Member

DPeterK commented Aug 3, 2018

Despite being a weighted aggregator, you can't actually pass weights to iris.analysis.MEAN when producing a lazy aggregation:

b = da.from_array(np.array([1,2,3,4,5]))
MEAN.lazy_aggregate(b, 0, weights=[2, 4, 1, 6, 3])
Traceback (most recent call last):
  File "/.../miniconda3/envs/scitools_dev/lib/python3.6/site-packages/IPython/core/interactiveshell.py", line 2963, in run_code
    exec(code_obj, self.user_global_ns, self.user_ns)
  File "<ipython-input-42-60626d069686>", line 1, in <module>
    MEAN.lazy_aggregate(b, 0, weights=[2, 4, 1, 6, 3])
  File "/.../git/iris/lib/iris/analysis/__init__.py", line 471, in lazy_aggregate
    return self.lazy_func(data, axis, **kwargs)
  File "/.../git/iris/lib/iris/analysis/__init__.py", line 1460, in inner_stat
    dask_result = dask_stats_function(array, axis=axis, **kwargs)
TypeError: mean() got an unexpected keyword argument 'weights'

This functionality is not tested by iris.tests.unit.analysis.test_MEAN.

One solution to this would be to use da.average rather than da.mean - but note there might be an implementational problem with da.average when the input array is masked.

@valeriupredoi
Copy link

hey guys. we have this issue documented in ESMValTool - with specific examples and work started by Ben towards a solution, if it helps https://github.com/ESMValGroup/ESMValTool/issues/887

@bouweandela
Copy link
Member

bouweandela commented Mar 28, 2019

It looks like dask.array.ma.average is now implemented:
https://github.com/dask/dask/blob/44948eca09b046cc06169dbdfc3cf4a5750a2598/dask/array/ma.py#L262-L264

so I think the solution could be as simple as replacing da.mean by da.ma.average here:

MEAN = WeightedAggregator('mean', ma.average,
lazy_func=_build_dask_mdtol_function(da.mean))

and pinning dask to an appropriately recent version, i.e. >= 1.1.0
http://docs.dask.org/en/latest/changelog.html#id16

@valeriupredoi
Copy link

valeriupredoi commented Mar 28, 2019

good find @bouweandela 🍺

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.

6 participants