NCAS-CMS / cf-python

A CF-compliant Earth Science data analysis library
http://ncas-cms.github.io/cf-python
MIT License
127 stars 19 forks source link

collapse('time: mean', group=cf.Y()) fails when cf.field() contains unused coordinates? #834

Open dlrhodson opened 1 week ago

dlrhodson commented 1 week ago

Hi Both!

I have a minor issue I encountered when trying to calculate time means when a field contains redundant X Y axes. Explanation and examples below.

Cheers, Dan


When collapsing a field by means along the time axis grouping into years, expect this to produce the annual means but this fails if the field contains unused X and Y coordinates. The attached example reproduces the issue for 3.16.2. The error does not occur in 3.15.1

The error I get is:

dlrhodso@sci8.jasmin.ac.uk monitor]$ ./cf_test1.py 3.16.2 Works for cfdata2.nc Traceback (most recent call last): File "/gws/nopw/j04/epoc/monitor/./cf_test1.py", line 16, in ann=data1.collapse('time: mean', group=cf.Y()) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/apps/jasmin/jaspy/miniforge_envs/jaspy3.11/mf3-23.11.0-0/envs/jaspy3.11-mf3-23.11.0-0-v20240815/lib/python3.11/site-packages/cf/decorators.py", line 71, in precede_with_kwarg_deprecation_check operation_method_result = operation_method(self, *args, kwargs) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/apps/jasmin/jaspy/miniforge_envs/jaspy3.11/mf3-23.11.0-0/envs/jaspy3.11-mf3-23.11.0-0-v20240815/lib/python3.11/site-packages/cfdm/decorators.py", line 171, in verbose_override_wrapper return method_with_verbose_kwarg(*args, *kwargs) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/apps/jasmin/jaspy/miniforge_envs/jaspy3.11/mf3-23.11.0-0/envs/jaspy3.11-mf3-23.11.0-0-v20240815/lib/python3.11/site-packages/cf/field.py", line 6931, in collapse f = f._collapse_grouped( ^^^^^^^^^^^^^^^^^^^^ File "/apps/jasmin/jaspy/miniforge_envs/jaspy3.11/mf3-23.11.0-0/envs/jaspy3.11-mf3-23.11.0-0-v20240815/lib/python3.11/site-packages/cfdm/decorators.py", line 171, in verbose_override_wrapper return method_with_verbose_kwarg(args, kwargs) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/apps/jasmin/jaspy/miniforge_envs/jaspy3.11/mf3-23.11.0-0/envs/jaspy3.11-mf3-23.11.0-0-v20240815/lib/python3.11/site-packages/cf/field.py", line 8480, in _collapse_grouped pc = self.subspace(**{axis: index}) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/apps/jasmin/jaspy/miniforge_envs/jaspy3.11/mf3-23.11.0-0/envs/jaspy3.11-mf3-23.11.0-0-v20240815/lib/python3.11/site-packages/cf/subspacefield.py", line 353, in call raise error File "/apps/jasmin/jaspy/miniforge_envs/jaspy3.11/mf3-23.11.0-0/envs/jaspy3.11-mf3-23.11.0-0-v20240815/lib/python3.11/site-packages/cf/subspacefield.py", line 348, in call out = field[indices]


  File "/apps/jasmin/jaspy/miniforge_envs/jaspy3.11/mf3-23.11.0-0/envs/jaspy3.11-mf3-23.11.0-0-v20240815/lib/python3.11/site-packages/cf/field.py", line 445, in __getitem__
    org_cyclic = [data_axes.index(axis) for axis in new.cyclic()]
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/apps/jasmin/jaspy/miniforge_envs/jaspy3.11/mf3-23.11.0-0/envs/jaspy3.11-mf3-23.11.0-0-v20240815/lib/python3.11/site-packages/cf/field.py", line 445, in <listcomp>
    org_cyclic = [data_axes.index(axis) for axis in new.cyclic()]
                  ^^^^^^^^^^^^^^^^^^^^^
ValueError: tuple.index(x): x not in tuple

But works for 3.15.1
./cf_test1.py
3.15.1
Works for cfdata2.nc
Works for cfdata1.nc

[cf_collapse_err.tar.gz](https://github.com/user-attachments/files/17814098/cf_collapse_err.tar.gz)

* The version of the software and the environment in which you are encountering an issue. The output of `cf.environment(paths=False)` is useful for this.
>>> cf.environment(paths=False)
Platform: Linux-3.10.0-1160.119.1.el7.x86_64-x86_64-with-glibc2.17
HDF5 library: 1.14.3
netcdf library: 4.9.2
udunits2 library: /apps/jasmin/jaspy/miniforge_envs/jaspy3.11/mf3-23.11.0-0/envs/jaspy3.11-mf3-23.11.0-0-v20240815/lib/libudunits2.so.0
esmpy/ESMF: 8.6.1
Python: 3.11.9
dask: 2024.8.0
netCDF4: 1.7.1
psutil: 6.0.0
packaging: 24.1
numpy: 1.26.4
scipy: 1.14.0
matplotlib: 3.8.4
cftime: 1.6.4
cfunits: 3.3.7
cfplot: 3.3.0
cfdm: 1.11.1.0
cf: 3.16.2
dlrhodson commented 1 week ago

This error does not occur in 13.6.1 ./cf_test1.py 3.16.1 Works for cfdata2.nc Works for cfdata1.nc

sadielbartholomew commented 6 days ago

Hi @dlrhodson, thanks for raising this and for all of the information provided. I see you've included the datasets in the tarball which is ideal.

If David hasn't looked at this already I'll try to take a look by the end of the week latest, thought somewhat preoccupied by co-delivering a training course so it will depend when I can find a quieter moment.

dlrhodson commented 6 days ago

No rush at all - it have a work around by using an earlier version of cf-python- but thought you might like to know about this edge case anyway!

sadielbartholomew commented 6 days ago

Dan, to confirm I've checked and the issue is still there for our current development branch, main, after many fixes have went in - I thought given the nature it would have been fixed by code we've since improved and patched but it turns out it has not.

@davidhassell, the line that errors is the one I raised as problematic before earlier in the year, originating from the Issue https://github.com/NCAS-CMS/cf-python/issues/756 where I suggested the direct fix #757 but we instead closed the Issue through the approach in https://github.com/NCAS-CMS/cf-python/issues/758, thinking it resulted from that as an upstream issue. But it seems like there are still cases where we emerge with at least one case whereby an axis in new.cyclic() which is not in data_axes, somehow, as per Dan's problem.

I might not have time to investigate until Friday at least since doing lots of the teaching at this course tomorrow, but it may be that we should re-open (or, effectively so) #757 and merge after all - or there may be some other upstream issue leading to an axis registered as cyclic not being in data_axes when it should be. If you have time to investigate please let me know what you think.